Page MenuHome

Theme: Radial gradient background and enum for gradient type
AcceptedPublic

Authored by Pablo Dobarro (pablodp606) on Wed, Feb 12, 7:24 PM.
Tokens
"Party Time" token, awarded by bblanimation."Pirate Logo" token, awarded by shader."Love" token, awarded by jonathanl."Love" token, awarded by rbx775."Love" token, awarded by Zino."Love" token, awarded by andruxa696."Party Time" token, awarded by psy-fi."Love" token, awarded by wilBr."Party Time" token, awarded by CobraA."Love" token, awarded by Brandon777."Love" token, awarded by Blendify.

Details

Summary

This patch replaces the "Use Gradient" checkbox theme option with an
enum and implements a radial background.

Whith this change, it should be easier to implemet other types of more
complex background types, like a world space oriented gradient.

This was a tweak I made in a custom build when I was working on the
sculpt branch and I used it to record some demo videos, but it was
highly requested to be included in master.

The patch works, but I'm not sure on how to implement the versioning and
the compatibility with previously saved themes.

Diff Detail

Repository
rB Blender
Branch
wb-radial-background (branched from master)
Build Status
Buildable 6595
Build 6595: arc lint + arc unit

Event Timeline

Clément Foucault (fclem) requested changes to this revision.Wed, Feb 12, 11:09 PM

Minor style comment but overall looks good to me.

source/blender/draw/engines/overlay/shaders/background_frag.glsl
65

style: use vectorized operation.
vec2 uv_n = uvcoordsvar.xy - 0.5;

66

I guess what you want here is 1 / (0.5 * sqrt(2)) (which is sqrt(2)). Use a define or a constant for it.

This revision now requires changes to proceed.Wed, Feb 12, 11:09 PM
Pablo Dobarro (pablodp606) marked 2 inline comments as done.
  • Review update
This revision is now accepted and ready to land.Thu, Feb 13, 1:28 PM

LGTM. I would also love for use to add a horizon-respecting gradient, but that's a separate thing.

I would rename "Gradient Colors" panel to be "Background" (and gradient_type to background_type). This would allow us in the future to use something else than a gradient, e.g. a texture such as a paper for 2D drawing (this is a bit overlapping with the per-viewport background settings though).

I always found it confusing that if you wanted to change the background color without a gradient, you'd have to change the Gradient High/Off property.

What do you think @William Reynish (billreynish)?

I guess ideally we should only show the Gradient Low property if it's valid. And when set to None, we could just call it Background.

Can I commit this as it is in the patch or should I rename the variables?
Also, doesn't this need any special versioning code to be compatible with the previously saved themes?