Anim: Theme entry for time visualization #120558

Merged
Christoph Lendenfeld merged 18 commits from ChrisLend/blender:theme_for_time_colors into main 2024-06-28 14:59:59 +02:00

This patch adds two theme entries for data before/after the current frame.
Those entries control what color to use when displaying reference data from a different point in time (motion path, onion skins).

They are under the 3D Viewport section of the theme in the user preferences.
image

- Before After
All image image
Annotations image image

User Facing Changes

Grease Pencil

When Custom Colors is disabled it will now use the theme instead of the layer default color.
Currently this setting is enabled by default. Disable it to get Blender to use the theme.

Motion Path

If Custom Color is disabled, read from the Theme.

Annotations

If onion skin is enabled, and custom onion skin colors is disabled, read from the theme.

Test Build

https://builder.blender.org/download/patch/PR120558/

For the Review

My reason for adding this setting into the theme is that it needs to match the background color. But that is just my reasoning.

Grease Pencil

  • previously the color used for onion skinning (without custom colors) was U.gpencil_new_layer_col. This is used in other places as well so it cannot be removed with this PR
  • By default, custom colors are enabled with Grease Pencil objects. I specifically did not change that with this PR. I will instead let the GP module handle that on their terms.

Based off this design task:
#119376: Theme colors for time visualization

This PR has been discussed in the Grease Pencil module meeting
and in the A&R module meeting

This patch adds two theme entries for data before/after the current frame. Those entries control what color to use when displaying reference data from a different point in time (motion path, onion skins). They are under the `3D Viewport` section of the theme in the user preferences. ![image](/attachments/fc63ecfb-5e9e-4746-b675-da9b5c449b5a) | - | Before | After | | - | - | - | | All | ![image](/attachments/313e7ddc-990d-4597-9197-ebba9e541694) | ![image](/attachments/f6d37fe0-78b9-47aa-8726-a091df331c37) | | Annotations | ![image](/attachments/eb842cb8-2e5f-4898-b661-e5dfe19ecca7) | ![image](/attachments/b059963c-0d64-4b83-acb0-9e5d8359e56e) | ## User Facing Changes ### Grease Pencil When `Custom Colors` is disabled it will now use the theme instead of the layer default color. Currently this setting is enabled by default. Disable it to get Blender to use the theme. ### Motion Path If Custom Color is disabled, read from the Theme. ### Annotations If onion skin is enabled, and custom onion skin colors is disabled, read from the theme. ### Test Build https://builder.blender.org/download/patch/PR120558/ ## For the Review My reason for adding this setting into the theme is that it needs to match the background color. But that is just my reasoning. **Grease Pencil** * previously the color used for onion skinning (without custom colors) was `U.gpencil_new_layer_col`. This is used in other places as well so it cannot be removed with this PR * By default, custom colors are enabled with Grease Pencil objects. I specifically did **not** change that with this PR. I will instead let the GP module handle that on their terms. ------ Based off this design task: [#119376: Theme colors for time visualization](https://projects.blender.org/blender/blender/issues/119376) This PR has been discussed in the [Grease Pencil module meeting](https://devtalk.blender.org/t/2024-05-21-grease-pencil-module-meeting/34755#meeting-notes-2) and in the [A&R module meeting](https://devtalk.blender.org/t/2024-05-24-animation-rigging-module-meeting/34813#patches-review-decision-time-4)
Christoph Lendenfeld added the
Module
Animation & Rigging
label 2024-04-12 10:48:46 +02:00
Christoph Lendenfeld added 2 commits 2024-04-12 10:48:59 +02:00
Christoph Lendenfeld added the
Interest
Grease Pencil
Interest
User Interface
labels 2024-04-12 10:49:17 +02:00
Christoph Lendenfeld added 1 commit 2024-04-12 11:50:39 +02:00
make annotations use the theme color
Some checks reported errors
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
b15dc63952
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR120558) when ready.
Christoph Lendenfeld added 1 commit 2024-04-12 13:08:37 +02:00
make gpencil use the theme colors
All checks were successful
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
414b4fec5e
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR120558) when ready.
Christoph Lendenfeld added 1 commit 2024-05-21 17:18:39 +02:00
Christoph Lendenfeld added 1 commit 2024-05-21 17:25:28 +02:00
add versioning
Some checks failed
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
65b2dd65b1
Christoph Lendenfeld changed title from Wip: Anim: Theme entry for time visualization to Anim: Theme entry for time visualization 2024-05-21 17:46:11 +02:00
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR120558) when ready.
Christoph Lendenfeld requested review from Nathan Vegdahl 2024-05-23 10:00:07 +02:00
Christoph Lendenfeld requested review from Falk David 2024-05-23 10:00:14 +02:00
Christoph Lendenfeld added 1 commit 2024-05-23 10:03:05 +02:00
Merge branch 'main' into theme_for_time_colors
All checks were successful
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
9756cb46aa
Author
Member

@blender-bot package

@blender-bot package
Member

Package build started. Download here when ready.

Package build started. [Download here](https://builder.blender.org/download/patch/PR120558) when ready.
Nathan Vegdahl requested changes 2024-05-23 10:23:37 +02:00
Dismissed
Nathan Vegdahl left a comment
Member

I'm not super familiar with this area of the code, but aside from one minor thing the code looks good to me. I'll build and try it out next.

I'm not super familiar with this area of the code, but aside from one minor thing the code looks good to me. I'll build and try it out next.
@ -45,3 +45,3 @@
else {
if (selected) {
intensity = calc_intensity(frameStart, frame, frameCurrent, 0.25, 0.75);
intensity = calc_intensity(frameStart, frame, frameCurrent, 0.0, 1.0);
Member

It looks like this if statement does the same thing in both branches now, so presumably it can be collapsed to just a single statement.

Same for the if statement a bit further down.

It looks like this `if` statement does the same thing in both branches now, so presumably it can be collapsed to just a single statement. Same for the `if` statement a bit further down.
Author
Member

thanks for pointing that out, in fact the intensity wasn't used at all anymore, so I removed all that code

thanks for pointing that out, in fact the intensity wasn't used at all anymore, so I removed all that code
nathanvegdahl marked this conversation as resolved
@ -2684,0 +2691,4 @@
prop = RNA_def_property(srna, "before_current_frame", PROP_FLOAT, PROP_COLOR_GAMMA);
RNA_def_property_array(prop, 4);
RNA_def_property_ui_text(
prop, "Before Current Frame", "The color for data that is from a frame before the current");
Member

Maybe "The color for things before the current frame (for onion skinning, motion paths, etc.)" and "The color for things after the current frame (for onion skinning, motion paths, etc.)".

I think the feature examples help make it more concrete.

Maybe "The color for things before the current frame (for onion skinning, motion paths, etc.)" and "The color for things after the current frame (for onion skinning, motion paths, etc.)". I think the feature examples help make it more concrete.
nathanvegdahl marked this conversation as resolved
Member

It works just as I would expect, except for one thing: in the theme editor you can set an alpha value for the colors, but that alpha is totally ignored by the actual drawing.

So either the alpha should be used in drawing, or it shouldn't be in the theme editor. I personally would prefer the former, as I think semi-transparent motion paths could be nice for not cluttering/obscuring the view too much, and same for setting a base level of transparency for onion skinning/ghosting.

It works just as I would expect, except for one thing: in the theme editor you can set an alpha value for the colors, but that alpha is totally ignored by the actual drawing. So either the alpha should be used in drawing, or it shouldn't be in the theme editor. I personally would prefer the former, as I think semi-transparent motion paths could be nice for not cluttering/obscuring the view too much, and same for setting a base level of transparency for onion skinning/ghosting.
Christoph Lendenfeld added 1 commit 2024-05-23 14:05:20 +02:00
Christoph Lendenfeld added 1 commit 2024-05-23 14:23:16 +02:00
Christoph Lendenfeld added 1 commit 2024-05-23 14:27:37 +02:00
Author
Member

It works just as I would expect, except for one thing: in the theme editor you can set an alpha value for the colors, but that alpha is totally ignored by the actual drawing.

I agree it would be nice to control the alpha value by the user preference. For this PR I'd like to keep the scope small though and do this in a separate step.
Also the user preferences is maybe not the right place for an alpha value. I can see the use case of adjusting that more often than the color

> It works just as I would expect, except for one thing: in the theme editor you can set an alpha value for the colors, but that alpha is totally ignored by the actual drawing. I agree it would be nice to control the alpha value by the user preference. For this PR I'd like to keep the scope small though and do this in a separate step. Also the user preferences is maybe not the right place for an alpha value. I can see the use case of adjusting that more often than the color
Contributor

I see properties_grease_pencil_common.py defines it for all annotations, that includes ones in other editors like Sequencer and UV. If we're storing this in theme (still not sold on that), wouldn't proper place for it be in "User Interface" panel instead of "3D Viewport"?

I see properties_grease_pencil_common.py defines it for all annotations, that includes ones in other editors like Sequencer and UV. If we're storing this in theme (still not sold on that), wouldn't proper place for it be in "User Interface" panel instead of "3D Viewport"?
Christoph Lendenfeld added 1 commit 2024-05-24 11:39:29 +02:00
Christoph Lendenfeld requested review from Nathan Vegdahl 2024-05-24 11:49:24 +02:00
Christoph Lendenfeld added 1 commit 2024-05-24 11:49:28 +02:00
Author
Member

I don't see a category for such an entry within "User Interface". The fact that this color is used in other editors is a good point though. Usually the entry gets added to those other editors as well then, which I personally dislike but it would be consistent with Blender.
@nathanvegdahl opinions?

I don't see a category for such an entry within "User Interface". The fact that this color is used in other editors is a good point though. Usually the entry gets added to those other editors as well then, which I personally dislike but it would be consistent with Blender. @nathanvegdahl opinions?
Member

Usually the entry gets added to those other editors as well then

That's my impression as well. So that's probably the way to go.

I agree, though, that that's rather unfortunate to split it up like that. But I think that's just part of the general unfortunate situation of how difficult it is to edit themes, which (as discussed in the module meeting) is a separate issue from whether or not this color setting logically belongs there or not.

It might be worth poking someone from the UI team about this, though, and see what they think makes the most sense. @pablovazquez ?

> Usually the entry gets added to those other editors as well then That's my impression as well. So that's probably the way to go. I agree, though, that that's rather unfortunate to split it up like that. But I think that's just part of the general unfortunate situation of how difficult it is to edit themes, which (as discussed in the [module meeting](https://devtalk.blender.org/t/2024-05-24-animation-rigging-module-meeting/34813)) is a separate issue from whether or not this color setting logically belongs there or not. It might be worth poking someone from the UI team about this, though, and see what they think makes the most sense. @pablovazquez ?
Christoph Lendenfeld added 2 commits 2024-05-28 10:46:18 +02:00
Author
Member

I've added the before and after colors to the Sequencer section of the Theme.
Turns out, not all editors that support annotations also support onion skinning. The sequencer is the only other editor that does.

I've added the before and after colors to the `Sequencer` section of the Theme. Turns out, not all editors that support annotations also support onion skinning. The sequencer is the only other editor that does.
Christoph Lendenfeld added 2 commits 2024-05-31 10:05:37 +02:00
Nathan Vegdahl approved these changes 2024-05-31 11:17:02 +02:00
Nathan Vegdahl left a comment
Member

Looks good to me! I'll poke the UI module in blender chat just to make sure they have visibility on it. But if we don't hear from them in a day or so then I think it's good to merge.

Looks good to me! I'll poke the UI module in blender chat just to make sure they have visibility on it. But if we don't hear from them in a day or so then I think it's good to merge.
Member

@ChrisLend Since it's been quite a while and no one from the UI module has chosen to weigh in, I think it's fine to land this.

@ChrisLend Since it's been quite a while and no one from the UI module has chosen to weigh in, I think it's fine to land this.
Author
Member

in that case I'll resolve the conflicts and merge today

in that case I'll resolve the conflicts and merge today
Christoph Lendenfeld added 1 commit 2024-06-28 14:13:42 +02:00
Christoph Lendenfeld added 1 commit 2024-06-28 14:21:45 +02:00
fix versioning
All checks were successful
buildbot/vexp-code-patch-lint Build done.
buildbot/vexp-code-patch-linux-x86_64 Build done.
buildbot/vexp-code-patch-darwin-arm64 Build done.
buildbot/vexp-code-patch-windows-amd64 Build done.
buildbot/vexp-code-patch-darwin-x86_64 Build done.
buildbot/vexp-code-patch-coordinator Build done.
6d5bf42a31
Author
Member

@blender-bot build

@blender-bot build
Christoph Lendenfeld merged commit 5427775fef into main 2024-06-28 14:59:59 +02:00
Christoph Lendenfeld deleted branch theme_for_time_colors 2024-06-28 15:00:03 +02:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
4 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#120558
No description provided.