Page MenuHome

UI: Custom Face Orientation Colors
ClosedPublic

Authored by Harley Acheson (harley) on Fri, Nov 15, 6:36 PM.
Tokens
"Like" token, awarded by item412."Y So Serious" token, awarded by shader."Like" token, awarded by fclem."Love" token, awarded by star."Love" token, awarded by Brandon777."100" token, awarded by CobraA."Like" token, awarded by pablovazquez."Love" token, awarded by amonpaike."Love" token, awarded by symstract."Love" token, awarded by Debuk."Love" token, awarded by Zino."Like" token, awarded by franMarz."Like" token, awarded by TheRedWaxPolice."Love" token, awarded by Christopher_Anderssarian.

Details

Summary

Not sure if we want this, but I've noticed it mentioned on "Blender Today" a couple of times.

This patch allows users to set custom colors for the "Face Orientation", rather than have it always set on red and blue:

I find it a little cool that you could set something bold and bright for the inward faces but select something completely transparent for outward-facing if you like.

Not sure who to set for code review, so not setting one for now.

Diff Detail

Repository
rB Blender

Event Timeline

Will update patch later today to use "front" and "back" in text and identifiers, rather than "out" and "in".

Very useful.

Now, when you say that we can "select something completely transparent for outward-facing", this means that we can have the front faces unchanged and only display a different color on the back faces?
If yes, that's great, and this could actually be enabled by default.

@TheRedWaxPolice (TheRedWaxPolice) - ...have the front faces unchanged and only display a different color on the back faces?

Yes, for both of these colors you can specify opacity. So you can set one to be completely transparent and therefore have no affect. Here with outward faces not colored:

If yes, that's great, and this could actually be enabled by default.

I'd assume that having the "Face Orientation" overlay enabled has some performance impact, so might be best to keep as an option. But not something for me to decide either way anyway.

Yes

Great.

I'd assume that having the "Face Orientation" overlay enabled has some performance impact

Too bad if that's the case. Most 3D apps have this feature enabled by default.

@TheRedWaxPolice (TheRedWaxPolice) - Too bad if that's the case. Most 3D apps have this feature enabled by default.

I honestly don't know how much impact it has. And it certainly could be enabled by default, and turned off by advanced users.

I'm just not personally involved in such decisions. That is for others to decide.

turned off by advanced users.

Actually, advanced users are the ones who would like to have it always enabled I think, haha... but with a very, very subtle color of course.
It's great to see at a glance the orientation of the faces. 🙂

I can see why this would be very useful. You could leave the inner color distinct, but the outer one neutral. Doesn't add a UI burden really either.

This revision is now accepted and ready to land.Sat, Nov 16, 1:42 AM

Now consistently using "front" and "back" for these different faces, rather than my "in" and "out" or current "towards" and "outwards".

Hey @William Reynish (billreynish), assuming you and @Pablo Vazquez (pablovazquez) like this I'd need to add a code reviewer. Can you suggest (or add) the most appropriate person?

I think Jeronen implemented this feature initially.

I think Jeronen implemented this feature initially.

Yes, but code review is slightly different. Once the feature is approved (William and Pablo in this case) the reviewer would be someone who would not only be able to check that I didn’t do anything dumb, but also with authority to allow me to commit it.

This revision now requires review to proceed.Sun, Nov 17, 10:31 AM
Jeroen Bakker (jbakker) requested changes to this revision.Mon, Nov 18, 9:08 AM

Hi @Harley Acheson (harley),

Patch overall looks fine, except that we are in the middle of a huge refactor; @Clément Foucault (fclem) is currently refactoring all the overlays as part of T65347: Unify overlay engines.
We should keep all changes on overlays to a minimum until the unification has been committed to master. That include this one, sorry. I don't have a clear vision on the ETA for the unification, but I expect it to be somewhere in the upcoming weeks.

In the end this means that the current patch will need some updates before it can land. These changes are minor as the structure of this patch is quite isolated but the I hope that the files will be organized differently :-).

Another task that needs to be done is the update of the current dark/light theme, but that can be tweaked outside this patch.

source/blender/draw/modes/overlay_mode.c
348 ↗(On Diff #19667)

Wrong place.
This will re-update the colors for every object in the scene. You could add this to where the ShadingGroup is constructed.

source/blender/draw/modes/shaders/overlay_face_orientation_frag.glsl
1 ↗(On Diff #19667)

Better to use common_globals_lib for this as it is already part of it.

Herefore the common_globals_lib needs to be added to the compilation of the face_orientation_shader. See overlay_mode#128 sh_data->face_orientation = GPU_shader_create_from_arrays.

source/blender/makesrna/intern/rna_userdef.c
1890

We use title case for labels

Also we should add clarification descriptions for every property. On second thought the UI themes are reused in many spaces so it should fit a more generic description.

1895

Same here

This revision now requires changes to proceed.Mon, Nov 18, 9:08 AM

Nice one! This gets asked every week, including a few hours ago today.

You can use blue/red as default so nothing changes at first, I will update the light/dark themes later.

Too bad if that's the case. Most 3D apps have this feature enabled by default.

Agreed, i think most of them only draw the inside faces ,maybe this is something could improve it's performance, if they can do it then Blender should also be able too.

@Pablo Vazquez (pablovazquez) - Nice one! This gets asked every week, including a few hours ago today.

Yes, hoped you'd like that.

You can use blue/red as default so nothing changes at first, I will update the light/dark themes later.

Yes, I made the current colors the defaults.

But, as noted by Jeroen Bakker above, it turns out that now is not really a good time for this as Clément is refactoring the overlays as part of this: https://developer.blender.org/T65347

So will revisit this when that work is done.

Updated to current state of master.

As per review by @Jeroen Bakker (jbakker) have done the following:

  • Settings of colors moved to where shading group is created
  • Fixed case of property labels

Have not (yet) figured out how to:

  • Use common_globals_lib for colors

If you need some hints, just poke me on blender.chat

@Jeroen Bakker (jbakker) - If you need some hints, just poke me on blender.chat

Thanks! Will say "hello" I see you online there.

In a nutshell I can see how to add "datatoc_common_globals_lib_glsl" to the frag shader when using GPU_shader_create_from_arrays(), as there are a couple of examples of that. And I also tried adding the "globalsBlock" to the shading group with DRW_shgroup_uniform_block().

Either way I can (usually) get it to compile, but when I directly reference an identifier in the globals it then just dies a terrible death. And then I am at the end of my knowledge as I am quite unfamiliar so far with the shader stuff. LOL

Now using colors from common_globals_lib.

kin malmin (star) rescinded a token.
kin malmin (star) awarded a token.

Redone to fit within @Clément Foucault (fclem)'s overlay refactor.

This revision is now accepted and ready to land.Sat, Dec 7, 9:44 AM
This revision was automatically updated to reflect the committed changes.