0
0
Fork 2

UI: Add polyline hide icon #2

Merged
Hans Goudey merged 1 commits from Sean-Kim/blender-assets:polyline-hide-icon into main 2024-04-10 13:59:31 +02:00
Contributor

This PR adds the ops.sculpt.polyline_hide icon.

Toolbar.blend In app
toolbar_blend_image_v2.png in_app_image_v2.png

Addresses icon for blender/blender#119483

Revision History

  • Replaced original icon by v2 made by @nickberckley
This PR adds the `ops.sculpt.polyline_hide` icon. | Toolbar.blend | In app | | --- | --- | | ![toolbar_blend_image_v2.png](attachments/c0313ef2-8207-44c7-bb47-102697bfeb3b) | ![in_app_image_v2.png](/attachments/1dec7f83-a683-44c5-9659-fea16e73d5de) | Addresses icon for blender/blender#119483 ## Revision History * Replaced original icon by v2 made by @nickberckley
Sean Kim added 1 commit 2024-03-21 21:03:11 +01:00
Sean Kim requested review from Pablo Vazquez 2024-03-21 21:03:27 +01:00
Sean Kim requested review from Daniel Bystedt 2024-03-21 21:03:41 +01:00
Member

Thanks for working on it! Discussed this a bit with @JulienKaspar , got some feedback:

  1. The white line thickness should match the other icons.
  2. There is probably too much detail for it to be seen in the small version.
  3. To make a connection with the regular Lasso tool, the overall polygonal shape could be closer to the lasso one.

Julien also mentioned that such polygonal tools are better described when the shape they make is a bit convex (which would be achieved with the third point above).

BTW, I'm leaving this feedback here so I don't forget, but I'm not expecting you to do design work that could be done by other artists in the module, you do enough coding the actual feature :D I'm a bit busy at the moment but could look into it soon-ish (after the release).

Thanks for working on it! Discussed this a bit with @JulienKaspar , got some feedback: 1. The white line thickness should match the other icons. 2. There is probably too much detail for it to be seen in the small version. 3. To make a connection with the regular Lasso tool, the overall polygonal shape could be closer to the lasso one. Julien also mentioned that such polygonal tools are better described when the shape they make is a bit convex (which would be achieved with the third point above). BTW, I'm leaving this feedback here so I don't forget, but I'm not expecting you to do design work that could be done by other artists in the module, you do enough coding the actual feature :D I'm a bit busy at the moment but could look into it soon-ish (after the release).
Member
  1. To make a connection with the regular Lasso tool, the overall polygonal shape could be closer to the lasso one.

Actually I meant that the shape should be more distinct. Even less like the lasso and box gesture tool shape. Sorry if that wasn't clear :D

A convex shape could help with making it more distinct.

> 3. To make a connection with the regular Lasso tool, the overall polygonal shape could be closer to the lasso one. Actually I meant that the shape should be more distinct. Even less like the lasso and box gesture tool shape. Sorry if that wasn't clear :D A convex shape could help with making it more distinct.
Contributor

How about
image

How about ![image](/attachments/ca133a0d-ebb7-4052-9e0f-b84125a0e96f)
Member

I like @nickberckley's V2, it has a nice amount of complexity and the shape looks sufficiently different from the lasso tool.

I like @nickberckley's V2, it has a nice amount of complexity and the shape looks sufficiently different from the lasso tool.
Member

@nickberckley How about
image

The last one, VII, communicates the function well and has a consistent width for lines and dots. +1 !

> @nickberckley How about > ![image](/attachments/ca133a0d-ebb7-4052-9e0f-b84125a0e96f) The last one, VII, communicates the function well and has a consistent width for lines and dots. +1 !
Contributor

@Sean-Kim if you like it as well I'll send you blend file. I rather not commit binary file myself.

@Sean-Kim if you like it as well I'll send you blend file. I rather not commit binary file myself.
Author
Contributor

@nickberckley Looks good to me as well, I can handle updating the PR if you send me the file!

@nickberckley Looks good to me as well, I can handle updating the PR if you send me the file!
Contributor

@Sean-Kim unfortunately I did this in old file before you changed order of icons, so I guess you'll need to move this icon in new one

@Sean-Kim unfortunately I did this in old file before you changed order of icons, so I guess you'll need to move this icon in new one
Sean Kim added 1 commit 2024-04-09 19:24:43 +02:00
Sean Kim force-pushed polyline-hide-icon from ab62aef163 to d566cf789d 2024-04-09 19:29:55 +02:00 Compare
Author
Contributor

@Sean-Kim unfortunately I did this in old file before you changed order of icons, so I guess you'll need to move this icon in new one

No worries, I've updated the file.

@pablovazquez - The PR should be in a good state to review again now!

> @Sean-Kim unfortunately I did this in old file before you changed order of icons, so I guess you'll need to move this icon in new one No worries, I've updated the file. @pablovazquez - The PR should be in a good state to review again now!
Member

Awesome! Thanks @Sean-Kim and @nickberckley !

Awesome! Thanks @Sean-Kim and @nickberckley !
Pablo Vazquez approved these changes 2024-04-10 12:21:32 +02:00
Hans Goudey approved these changes 2024-04-10 13:59:10 +02:00
Hans Goudey merged commit 597eb051ea into main 2024-04-10 13:59:31 +02:00
Sign in to join this conversation.
No Label
No Milestone
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender-assets#2
No description provided.