VSE: Scopes improvements #116798

Merged
Aras Pranckevicius merged 13 commits from aras_p/blender:vse-scopes into main 2024-01-05 22:03:13 +01:00

Several things about VSE Scopes:

  • Improve the look of them, so they feel less like from year 1998 (images below).
  • Some of the scopes got slightly faster in the process, others stayed the same performance (details below).
  • Remove VSE Scopes related data from SpaceSeq DNA, move it into runtime instead. And the runtime part, make it by-pointer in SpaceSeq, similar to how other spaces do it. No need to "leak" internal runtime struct definitions all the way up to DNA. DNA_space_types.h no longer includes DNA_sequence_types.h now, so the latter had to be added to a handful of source files that actually need it.
  • General VSE scopes code duplication/cleanup and replacing some C threading parts with C++ ones etc. Net result is that there's less code now, yay.

Performance numbers everywhere below are with VSE at 4K UHD resolution, with a single 4K resolution PNG image strip. Playback FPS on Ryzen 5950X, Windows/VS2022 build.

Where behavior/look is somewhat different, I'm including both "byte" (PNG) image and "float" (EXR) image cases.

Show Overexposed (zebra) overlay

  • For byte images, it was inverting colors in overexposed areas. However for float images, it was turning colors negative and clamping them, effectively making "zebra" parts always black. That sounded like a bug, so I unified the behavior and for float images now that also inverts the colors.

Luma Waveform

  • Adjust intensity of how fast waveform samples accumulate. I've tried adjusting it so it is not too dark nor saturates too much, based on vertical render resolution (each column of input image distributes into fixed number of intensity "bins", so the higher the vertical res, the more samples possibly accumulate into the same bins). Result is that it does not saturate as much as before, allowing to see more detail.
  • Draw outline/grid on the GPU, and draw text labels indicating horizontal grid line levels (0.1, 0.7, 0.9).
  • Performance a bit faster: 39 -> 46 FPS.

Before, PNG: wave-png-before.png
After, PNG: wave-png-after.png

Before, EXR: wave-exr-before.png
After, EXR: wave-exr-after.png

Luma Waveform, Separated colors ("Parade")

  • Same treatment as above.
  • Also make the colors not completely saturated RGB, but somewhat easier on the eyes.
  • Performance: 29 -> 38 FPS.

Before, PNG: parade-png-before.png
After, PNG: parade-png-after.png

Chroma Vectorscope

  • Scope is displayed preserving aspect ratio, previously was stretched based on render aspect (for very wide aspects it looked really weird).
  • RGB -> YUV conversion is done with standard BT709 coefficients, instead of using something that was close but not quite (possibly whatever standard existed 15 years ago?).
  • Changed the look of the graticule: now it's drawn on the GPU, and draws area from the full saturation (outer line as it was previously already), to the inner "safe 75% saturation" where most other tools display primary color points.
  • Also shows "skin tone" line, just like is done in the blender image scopes.
  • Performance seems to be a tiny bit faster: 22 -> 24 FPS.

Before, PNG: vecscope-png-before.png
After, PNG: vecscope-png-after.png

Histogram

  • Draw histogram as filled area & top line on the GPU, instead of very low-res image.
  • Make the RGB parts less than "fully saturated, in your face" colors.
  • Draw vertical grid lines indicating several intensity levels, along with text labels. Note: for float (e.g. EXR) images the previous histogram already had two small "spokes" near the top, that looked like some sort of bug, but they were actually indicating 0.0 and 1.0 levels (since for float images, histogram displays -0.25..+1.25 range). Now with lines and labels that is more clear.
  • Performance seems to be a tiny bit faster: 53 -> 57 FPS.

Before, PNG: hist-png-before.png
After, PNG: hist-png-before.png

Before, EXR: hist-png-before.png
After, EXR: hist-png-before.png

Blender Image Scopes

For comparison, the scopes from Blender's own "image" space below. At first I thought I'd try to factor out that code and make it shared between image and vse, but gave up on trying to do that. The image space scopes displays are "special type of UI button" deep within UI code, and my familiarity with that is very lacking, and I could not find a way to factor out the data they display out of DNA structs (which sounds like completely wrong place to hold it, but it does today). So for now, I have not touched the image scopes code at all.

PNG: hist-png-before.png
EXR: hist-png-before.png

Several things about VSE Scopes: - Improve the look of them, so they feel less like from year 1998 (images below). - Some of the scopes got slightly faster in the process, others stayed the same performance (details below). - Remove VSE Scopes related data from SpaceSeq DNA, move it into runtime instead. And the runtime part, make it by-pointer in SpaceSeq, similar to how other spaces do it. No need to "leak" internal runtime struct definitions all the way up to DNA. `DNA_space_types.h` no longer includes `DNA_sequence_types.h` now, so the latter had to be added to a handful of source files that actually need it. - General VSE scopes code duplication/cleanup and replacing some C threading parts with C++ ones etc. Net result is that there's _less_ code now, yay. Performance numbers everywhere below are with VSE at 4K UHD resolution, with a single 4K resolution PNG image strip. Playback FPS on Ryzen 5950X, Windows/VS2022 build. Where behavior/look is somewhat different, I'm including both "byte" (PNG) image and "float" (EXR) image cases. ### Show Overexposed (zebra) overlay - For byte images, it was inverting colors in overexposed areas. However for float images, it was turning colors negative and clamping them, effectively making "zebra" parts always black. That sounded like a bug, so I unified the behavior and for float images now that also inverts the colors. ### Luma Waveform - Adjust intensity of how fast waveform samples accumulate. I've tried adjusting it so it is not too dark nor saturates too much, based on vertical render resolution (each column of input image distributes into fixed number of intensity "bins", so the higher the vertical res, the more samples possibly accumulate into the same bins). Result is that it does not saturate as much as before, allowing to see more detail. - Draw outline/grid on the GPU, and draw text labels indicating horizontal grid line levels (0.1, 0.7, 0.9). - Performance a bit faster: 39 -> 46 FPS. Before, PNG: ![wave-png-before.png](/attachments/b8cf7287-a3f4-40dc-8bdb-effe4ecde7e8) After, PNG: ![wave-png-after.png](/attachments/00c25d0c-aaf4-4839-8a0a-85dcb7c63f8b) Before, EXR: ![wave-exr-before.png](/attachments/0d40b6a4-c3c8-45c6-a7c0-082a48d21608) After, EXR: ![wave-exr-after.png](/attachments/a41e5d3b-dc1a-41bf-88b4-b3848c12bf77) ### Luma Waveform, Separated colors ("Parade") - Same treatment as above. - Also make the colors not completely saturated RGB, but somewhat easier on the eyes. - Performance: 29 -> 38 FPS. Before, PNG: ![parade-png-before.png](/attachments/2ff52759-ea64-442f-b8ab-c4a421febeb2) After, PNG: ![parade-png-after.png](/attachments/eb42565f-544e-4d36-9c0b-c03c087151d2) ### Chroma Vectorscope - Scope is displayed preserving aspect ratio, previously was stretched based on render aspect (for very wide aspects it looked really weird). - RGB -> YUV conversion is done with standard BT709 coefficients, instead of using something that was close but not quite (possibly whatever standard existed 15 years ago?). - Changed the look of the graticule: now it's drawn on the GPU, and draws area from the full saturation (outer line as it was previously already), to the inner "safe 75% saturation" where most other tools display primary color points. - Also shows "skin tone" line, just like is done in the blender image scopes. - Performance seems to be a tiny bit faster: 22 -> 24 FPS. Before, PNG: ![vecscope-png-before.png](/attachments/ca7e70eb-70b9-41f4-8981-11c545965997) After, PNG: ![vecscope-png-after.png](/attachments/83aa4dd9-5320-48b0-9678-01f909d0f636) ### Histogram - Draw histogram as filled area & top line on the GPU, instead of very low-res image. - Make the RGB parts less than "fully saturated, in your face" colors. - Draw vertical grid lines indicating several intensity levels, along with text labels. Note: for float (e.g. EXR) images the previous histogram already had two small "spokes" near the top, that looked like some sort of bug, but they were actually indicating 0.0 and 1.0 levels (since for float images, histogram displays -0.25..+1.25 range). Now with lines and labels that is more clear. - Performance seems to be a tiny bit faster: 53 -> 57 FPS. Before, PNG: ![hist-png-before.png](/attachments/b2101b35-d9ad-461f-b277-397a5fbf0486) After, PNG: ![hist-png-before.png](/attachments/0df12eba-7b2c-4181-9797-dfcd5109aac6) Before, EXR: ![hist-png-before.png](/attachments/2e18992b-5de2-46ff-a344-40d7f9e9b196) After, EXR: ![hist-png-before.png](/attachments/7da4eec0-3f3a-4d04-8492-704ca5a799de) ### Blender Image Scopes For comparison, the scopes from Blender's own "image" space below. At first I thought I'd try to factor out that code and make it shared between image and vse, but gave up on trying to do that. The image space scopes displays are "special type of UI button" deep within UI code, and my familiarity with that is very lacking, and I could not find a way to factor out the data they display out of DNA structs (which sounds like completely wrong place to hold it, but it does today). So for now, I have not touched the image scopes code at all. PNG: ![hist-png-before.png](/attachments/b4dd4c3a-6062-4d49-bdcf-cdcbdfe5788c) EXR: ![hist-png-before.png](/attachments/3d9e54b2-d53c-4956-b130-e9ab02a08448)
Aras Pranckevicius added 7 commits 2024-01-04 20:04:48 +01:00
6767dc2b39 VSE: improve Vectorscope
- RGB -> YUV conversion uses BT709 (just like image vectorscope),
  instead of "something similar but not quite".
- It now has square aspect ratio.
- Shows skin tone line, just like image vectorscope.
- Shows full saturation and "safe" saturation range as filled area,
  drawing that on the GPU instead of poking in the full saturation
  ring on the CPU into the texture.
099033fc57 VSE: overexposed/zebra overlay consistency between byte and float images
Byte is showing inverted (white-color) overlay, whereas float was
showing overlay as black stripes. Goes back to 16 years ago when VSE
code was added. Looks like an accident, made both show inverted colors.
Aras Pranckevicius added 2 commits 2024-01-04 21:17:51 +01:00
d52353c550 VSE: tweak vectorscope perf and look
Non-inlined call to rgb_to_yuv was costing a lot of perf, do the
calculations inline with a note.

Also tweak the look to scale U,V non-uniformly, similar to previous
scope behavior. Uses the horizontal texture resolution better.
05927be070 VSE: fix new histogram code not being multi-threaded
New code uses C++ threading::parallel_reduce, but that requires WITH_TBB
to be set on cmake side.
Aras Pranckevicius added 1 commit 2024-01-04 22:23:33 +01:00
Aras Pranckevicius added 1 commit 2024-01-05 10:04:05 +01:00
buildbot/vexp-code-patch-lint Build done. Details
buildbot/vexp-code-patch-darwin-x86_64 Build done. Details
buildbot/vexp-code-patch-linux-x86_64 Build done. Details
buildbot/vexp-code-patch-windows-amd64 Build done. Details
buildbot/vexp-code-patch-darwin-arm64 Build done. Details
buildbot/vexp-code-patch-coordinator Build done. Details
7e162aff29
Merge branch 'main' into vse-scopes
Author
Member

@blender-bot build

@blender-bot build
Aras Pranckevicius added this to the Video Sequencer project 2024-01-05 10:58:58 +01:00
Aras Pranckevicius changed title from WIP: VSE: Scopes improvements to VSE: Scopes improvements 2024-01-05 10:59:16 +01:00
Aras Pranckevicius requested review from Richard Antalik 2024-01-05 17:23:01 +01:00
Hans Goudey reviewed 2024-01-05 18:18:29 +01:00
Hans Goudey left a comment
Member

+1! I just commented on some superficial things when reading the PR

+1! I just commented on some superficial things when reading the PR
@ -43,0 +53,4 @@
blender::ed::seq::SeqScopes scopes;
SpaceSeq_Runtime() = default;
SpaceSeq_Runtime(const SpaceSeq_Runtime &) = delete;
Member

BLI_utility_mixins.hh makes this a bit nicer, with NonCopyable

`BLI_utility_mixins.hh` makes this a bit nicer, with `NonCopyable`
aras_p marked this conversation as resolved
@ -0,0 +30,4 @@
void cleanup();
ImBuf *reference_ibuf = nullptr;
Member

The style guide mentions putting member variables before method declarations, might as well stay consistent

The style guide mentions putting member variables before method declarations, might as well stay consistent
aras_p marked this conversation as resolved
@ -76,2 +75,4 @@
typedef struct SpaceFile_Runtime SpaceFile_Runtime;
/** Defined in `sequencer_intern.hh`. */
typedef struct SpaceSeq_Runtime SpaceSeq_Runtime;
Member

What about putting the runtime struct into the editor's namespace, like the node and outliner runtime structs?

What about putting the runtime struct into the editor's namespace, like the node and outliner runtime structs?
aras_p marked this conversation as resolved
Richard Antalik reviewed 2024-01-05 19:07:52 +01:00
Richard Antalik left a comment
Member

Nice patch! I have mostly nitpicky comments again, but wanted to mainly clarify usage of overlays here as that is not clear to me how it works actually.

Nice patch! I have mostly nitpicky comments again, but wanted to mainly clarify usage of overlays here as that is not clear to me how it works actually.
@ -572,0 +537,4 @@
float grid_x_0 = area.xmin;
float grid_x_1 = area.xmax;
/* Float histograms show -0.25 .. 1.25 area horizontally. */
if (hist.data.size() > 256) {

This looks a little bit cursed, along with (0.25f / 1.5f). It wasn't obvious to me, that float images do use 512 wide array and that this oversaples the image. Can you clarify this in a comment?

This looks a little bit cursed, along with `(0.25f / 1.5f)`. It wasn't obvious to me, that float images do use 512 wide array and that this oversaples the image. Can you clarify this in a comment?
Author
Member

Good point, will add named constants for these things

Good point, will add named constants for these things
aras_p marked this conversation as resolved
@ -613,2 +545,2 @@
scope = scopes->histogram_ibuf;
break;
View2D *v2d = &region->v2d;
const float text_scale_x = BLI_rctf_size_x(&v2d->cur) / BLI_rcti_size_x(&v2d->mask);

You can use UI_view2d_scale_get_x() here

You can use `UI_view2d_scale_get_x()` here
aras_p marked this conversation as resolved
@ -615,0 +553,4 @@
/* Label. */
char buf[10];
snprintf(buf, sizeof(buf), "%.2f", val);

Use BLI_snprintf()

Use `BLI_snprintf()`
aras_p marked this conversation as resolved
@ -623,0 +616,4 @@
for (int i = 0; i < 3; i++) {
const float y = area.ymin + (area.ymax - area.ymin) * lines[i];
char buf[10];
snprintf(buf, sizeof(buf), "%.1f", lines[i]);

Use BLI_snprintf()

Use `BLI_snprintf()`
aras_p marked this conversation as resolved
@ -623,0 +681,4 @@
/* skin tone line */
uchar col_tone[4] = {255, 102, 0, 128};
const float tone_line_len = 0.895f; /* makes it end at outer edge of saturation ring */

I mean, it is a solution :) Just to be clear, no need to do anything about this, just wanted to say, that I laughed a bit.
Eh actually, I noticed a missing full stop at the comment, so that should be fixed.

I mean, it is a solution :) Just to be clear, no need to do anything about this, just wanted to say, that I laughed a bit. Eh actually, I noticed a missing full stop at the comment, so that should be fixed.
aras_p marked this conversation as resolved
@ -623,0 +698,4 @@
sequencer_preview_get_rect(&preview, scene, region, sseq, draw_overlay, false);
rctf uv;
if (draw_overlay && (sseq->overlay_frame_type == SEQ_OVERLAY_FRAME_TYPE_RECT)) {

I am bit lost here, why do you need overlay rect for scope?

I am bit lost here, why do you need overlay rect for scope?
Author
Member

Nice catch! I don't.

Nice catch! I don't.
aras_p marked this conversation as resolved
@ -566,4 +284,3 @@
return 511;
}
return int(((f + 0.25f) / 1.5f) * 512);

Perhaps round_fl_to_int() would be bit more precise.

Perhaps `round_fl_to_int()` would be bit more precise.
Author
Member

Rounding would be wrong as-is, since it would result in possible index 512 which is just past the end of the array.

Rounding would be wrong as-is, since it would result in possible index 512 which is just past the end of the array.
aras_p marked this conversation as resolved
Aras Pranckevicius added 2 commits 2024-01-05 20:25:57 +01:00
Richard Antalik approved these changes 2024-01-05 21:17:29 +01:00
Aras Pranckevicius merged commit 423e54b000 into main 2024-01-05 22:03:13 +01:00
Aras Pranckevicius deleted branch vse-scopes 2024-01-05 22:03:17 +01:00
Sign in to join this conversation.
No reviewers
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
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
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 Assignees
3 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#116798
No description provided.