Image Editor Vectorscope Improvement #116974
No reviewers
Jeroen Bakker
Pablo Vazquez
Aras Pranckevicius
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Code Documentation
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
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
Viewport & EEVEE
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
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
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
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
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#116974
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "JonasDichelle/blender:vectorscope-update"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Design Issue: #116973
Image Editor Vector Scope Improvement
Overview
The vectorscope in the image editor currently has several issues, mostly in terms of readability.
I've made some updates to the code in an attempt to improve the UI.
Key Issues
Readability:
Color Representation:
Aesthetic Concerns:
The Proposal
The code changes I have made tackle the issues I've outlined above.
The Color Point Cloud
RGBmode to the scope, which colors the points, according to the color they represent, the old display mode is still accessible through theLumaoption.Vectorscope Opacityaffects the opacity of the individual points allowing the user to find areas where certain colors are more concentrated.The Scope
Demonstration
Demonstration of the usability improvements of the update
WIP Image Editor Vectorscope Improvementto WIP: WIP Image Editor Vectorscope ImprovementWIP: WIP Image Editor Vectorscope Improvementto WIP: Image Editor Vectorscope Improvement@ -166,2 +168,2 @@int waveform_tot;char _pad[4];float *vecscope_rgb;char _pad[8]; // Adjusted for alignmentIs the
_padneeded at all now?good catch thanks!
@ -1763,6 +1763,8 @@ void DRW_draw_render_loop_ex(Depsgraph *depsgraph,drw_engines_draw_scene();// Clean upLooks like leftover comment.
@ -956,2 +951,2 @@polar_to_x(centerx, diam, tampli + dampli, tangle - dangle + dangle2),polar_to_y(centery, diam, tampli + dampli, tangle - dangle + dangle2));//draw textIn general blender uses C style comments, i.e.
/* foo */not// foothanks for the hint!
I have recently done a "facelift" for the video sequence editor scopes -- #116798 -- do you think it would make sense to unify the "look" of image & sequencer scopes maybe?
@aras_p, I like the intensity accumulation changes you made, they help readability a lot.
It would be good to implement those in the image editor's histogram and waveform too.
I also think the inconsistency between the scopes is something to think about. The way the appearance of the VSE scopes is dependent on the dimensions of the scene feels weird to me.
I think it would probably be best to have the scopes also be in the side panel as they are in the image editor.
WIP: Image Editor Vectorscope Improvementto Image Editor Vectorscope ImprovementI added Pablo as reviewer as it is UI/UX related and with recent changes the vector scopes seems to be different between image editor/sequencer. So perhaps we first need to settle on the right UI/UX (this patch feels promising, but I am not the expert on using vector scopes).
I am also not aware why the vector scopes are separated. Can imagine that this is legacy. If so we should check what effort/risk is needed to combine both. This could be done in a separate PR, before or after this PR lands.
@Jeroen-Bakker FWIW I tried to look into sharing the scopes code between Image and Sequencer a few weeks ago, and gave up. The image scopes have their own special UI widget type to draw themselves, which makes it really cumbersome to take out the image scopes data out of DNA (which arguably should never have been part of DNA to begin with!). I'd be all for unifying the look (and the code, if possible) between them, but due to how the image scopes drawing is structured right now, it's not exactly trivial :/
@Jeroen-Bakker thank you for taking a look at this issue. The code design of the scopes could definitely do with an overhaul and I agree with @aras_p that that would be a pretty big undertaking.
Oooh nice! I think the outer ring could be a bit thicker (if possible, I know we have OpenGL limits).
For the Luma version (or both?), perhaps having the background of the scope be a darkened version of the colorwheel. What do you think?
I've made a quick mockup in GIMP:
@pablovazquez, thank you for the feedback!
Changing the line width is pretty easy. Here's a screenshot with
GPU_line_width(2.0f):I've also increased the opacity to 100% (might be too much), previously it was at 70%.
Making the background have an accurately filled in color wheel would require writing a new OpenGL shader, which would take a bit longer, though.
Neat! I think the wider line makes it look nicer and serve its purpose better.
Would it help to add
GPU_line_smooth(true);in the mix for better anti aliasing? Or am I wishful thinking here. Thanks!Oh wow, that look a lot better!
I like the way it looks a lot now.
It does not necessarily need a new shader, you could draw a triangle fan with vertex colors, and use the existing "with vertex colors, smooth interpolation" shader. More triangles than scribbled here, but you get the idea:
@aras_p, I've thought about that too. But in that case, for example, you wouldn't have an accurate loss of saturation towards the center because the color would change linearly towards the center on each radius.
To accurately represent the colors in the background, I believe, you would need some sort of new fragment shader.
For this vectorscope specifically it didn't mean to be super accurate but dimmed in the background as a hint.
If you (+Aras) plan to improve scopes code and design in general we could re-visit this again in the future as part of a more unified look.
This already is a great improvement for 4.1, so in regards to design: +1 !
Yes, I just tested it and was going to add that it actually looks quite good even if not entirely accurate and might be good enough.
Thanks for approving!
Ooh much nicer indeed! My +1 goes to this one now hah.
Thank you for having a look at this far corner of Blender :D
Cool, I pushed the new version.
I only added it to the luma mode since it looked really bad together with the colored points.
Thanks! I'll leave this then for @Jeroen-Bakker or @aras_p to review the code.
@pablovazquez should I put the same visual look into VSE vectorscope? (I think that would be good to do)
That'd be awesome! First step towards a unified look.
@ -976,2 +972,4 @@{0.75, 0.0, 0.75}, /* Magenta */};const char color_names[6][4] = {"R", "Y", "G", "C", "B", "M"};Minor: not sure why color names are array of 4 characters each, when they are all a single char. Maybe change them to not be strings at all, and make vectorscope_draw_target just take a single char for the letter? (and internally have a
char[2]buffer for passing to font drawing functions)Good call, I'll change that!
i updated it in
2bebddf117@ -1016,0 +1099,4 @@circle_draw_rgb(circle_points, tot_points, circle_vertex_colors, GPU_PRIM_LINE_LOOP);GPU_line_width(1.5f);/* inner circles */Should the inner circles also use
circle_draw_rgbmachinery, instead of immediate mode?Yes we can do that, updated it in the latest commit!
@ -1362,0 +1363,4 @@scopes->vecscope_rgb[color_idx + 0] = rgb[0];scopes->vecscope_rgb[color_idx + 1] = rgb[1];scopes->vecscope_rgb[color_idx + 2] = rgb[2];scopes->vecscope_rgb[color_idx + 3] = scopes->vecscope_alpha;Given that
vecscope_rgbalways uses the same single value for the alpha component of each point, would it be worth changing it to allocate 3 floats per sample instead of 4? Would save some memory, and at draw time could just fetch the alpha fromvecscope_alphafield.I did this because we need to pass in 4 values for each Vertex into the shader.
Having individual alphas for each vertex is necessary to get good looking accumulation.
If we didn't do this here we would need to add the alpha values to the array at a later point.
The only way to avoid this would be to create a new shader.
But please correct me if I'm wrong.
Ah, I see, this array is directly copied into the GPU vertex buffer. Ignore me then!
@ -1147,1 +1147,4 @@static const EnumPropertyItem prop_vecscope_mode_items[] = {{SCOPES_VECSCOPE_LUMA, "LUMA", ICON_COLOR, "Luma", ""},{SCOPES_VECSCOPE_RGB, "RGB", ICON_COLOR, "Red Green Blue", ""},"Red Green Blue" as a UI label feels a bit weird (and hardly fits into UI control space). Maybe "RGB" or "Color" instead?
I had the same issue with this but looking at all other scopes they use
Red Green Blue. This patch is just following what's already there.Once this gets in we can do a pass and rename that for all scopes. I'd also like to make a few changes in that UI later for all scopes (the opacity slider after the type selector, leave just "opacity" in the label so it's less verbose, remove icon from enum, etc).
I agree RGB would be better.
I just copied the label from the Waveform which has "Red Green Blue".
But I'm also in favor of changing this.
@pablovazquez @JonasDichelle hey look! #117738
@aras_p Very cool, it looks a lot better especially compared to 4.0!
The code looks alright to me. Might be worth rebasing or merging latest main though.
Done, thanks for reviewing it!
@blender-bot build
@blender-bot build
Andrea Monzini referenced this pull request2024-02-14 00:19:23 +01:00