Patch for: T37878
Details
Diff Detail
- Repository
- rB Blender
Event Timeline
One question I have is about UI representation of toggle button to turn on/off this behavior. In this patch I implemented checkbox with text, but there is also possibility of less cluttered icon toggle button to the right of the normals size slider.
I like the clutter less approach. But screen space is a bit too technical even for a tooltip. I would suggest Constant Screen Size or Constant Screen Length. I'm quite bad at naming myself. Maybe @Campbell Barton (campbellbarton) (because it touches the modeling) , @Brecht Van Lommel (brecht) or @Pablo Vazquez (pablovazquez) have a better idea.
| source/blender/draw/engines/overlay/shaders/edit_mesh_normal_vert.glsl | ||
|---|---|---|
| 59 | Instead of this, I would create a new value for the screen space scale and make it a PROP_PIXEL value in DNA. This way toggling between both does not become anoying. | |
"Constant Screen Size" seems reasonable. Would not use "length" since it's already named "size" and that's the term we use for other viewport preferences.
I think an icon is preferable, but maybe the UI team can help find the best icon to use or add a new one.
- Improved naming for code and UI
- Changed UI to be less cluttered (icon used in this patch is not a final solution though)
Regarding the icon
I created my version of an icon to be used in this patch, unfortunately I didn't manged to include it as whenever I saved the blender_icons.svg file my Inkscape version was changing a lot of stuff all over the place in the SVG/XML text file and I think this was affecting the blender_icons_update.py script as it wasn't adding a new *.dat file for the icon but also was changing some other random ones.
First things first - UI/UX
@Julian Eisel (Severin), can somebody from the UI team can give "go/no go" for this UI/UX approach presented in this patch (video showing it is in description of this diff)? If this solution will have the blessing from the UI team then I would propose to submit the icon in the separate diff (before this one). Of course as discussed with @Julian Eisel (Severin) this new icon would need to be accepted by @Andrzej Ambroz (jendrzych), but we can talk about the details in private. My current version looks like this (in the center):
Regarding the design: Personally I too prefer having it as an icon, simply to make it take less space. Although it's certainly useful, it's not a vital option, so having it nicely compact makes sense.
@Piotr Makal (pmakal) yeah, unfortunately Inkscape always does change a lot of stuff in the SVG. This stuff can be manually removed from the patch (e.g. by only adding relevant bits to the commit using git add -p), but it's a hassle. And it's not always clear what's related and what not... (usually it's harmless though, it's just a bit of noise in the patch.)
Also unfortunately, there seem to be version differences in Inkscape that generate different .dat files. Just use git restore path/to/a/file.dat for the unrelated ones.
Hey - this button cries for more generic and universal "fixed size" icon, that can be easily reused enywhere else in the GUI.
(just ignore the right one - it's way to similar to an "X"=delete)
The patch is good feature wise. I just have a few style comment.
| source/blender/draw/engines/overlay/shaders/edit_mesh_normal_vert.glsl | ||
|---|---|---|
| 58–59 | Do not put comment this ways. bool is_persp = (ProjectionMatrix[3][3] == 0.0);
if (is_persp) { | |
| source/blender/makesdna/DNA_view3d_types.h | ||
| 559 | Rename to V3D_OVERLAY_EDIT_CONSTANT_SCREEN_SIZE_NORMALS. | |
| source/blender/makesrna/intern/rna_space.c | ||
| 4369 | Rename to normals_constant_screen_size. This way it is easier to find with auto complete. | |
- Merged to latest master
- Cleanup: Renamed normals_screen_length to normals_constant_screen_size.
- Add versioning code for normals_constant_screen_size.



