griefith/test2
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix #125061: VSE timeline pixel rounding/snapping issues at large frames

Browse Source 
VSE timeline widget drawing is done in "timeline space" (x: frames,
y: channels), but that can have precision issues at large frames,
when "pixel size features" (outlines, borders) need to get evaluated
inside a shader.

This can lead to inconsistent border sizes between neighboring strips,
e.g. sometimes it would be 2 pixels, but sometiems 3 pixels. I've seen
this mostly happen when frames get into 100'000+ range.

To address this, switch timeline widget drawing to be in window pixel
space. This avoids the issue since coordinates to draw the strip
widgets become "up to several thousand" range, not arbitrarily large.

Pull Request: https://projects.blender.org/blender/blender/pulls/125220
This commit is contained in:
Aras Pranckevicius
2024-07-22 16:08:07 +02:00
committed by Aras Pranckevicius
parent dcef32ec9b
commit 527e55239b
7 changed files with 67 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
source/blender/editors/space_sequencer/sequencer_drag_drop.cc
View File

@@ -377,7 +377,7 @@ static void draw_seq_in_view(bContext *C, wmWindow * /*win*/, wmDrag *drag, cons
float strip_len = update_overlay_strip_position_data(C, mval);
GPU_matrix_push();
UI_view2d_view_ortho(&region->v2d);
wmOrtho2_region_pixelspace(region);
/* Sometimes the active theme is not the sequencer theme, e.g. when an operator invokes the
* file browser. This makes sure we get the right color values for the theme. */
@@ -401,7 +401,7 @@ static void draw_seq_in_view(bContext *C, wmWindow * /*win*/, wmDrag *drag, cons
float pixelx = BLI_rctf_size_x(&region->v2d.cur) / BLI_rcti_size_x(&region->v2d.mask);
float pixely = BLI_rctf_size_y(&region->v2d.cur) / BLI_rcti_size_y(&region->v2d.mask);
StripsDrawBatch batch(pixelx, pixely);
StripsDrawBatch batch(&region->v2d);
for (int i = 0; i < coords->channel_len; i++) {
float y1 = floorf(coords->channel) + i + SEQ_STRIP_OFSBOTTOM;

32
source/blender/editors/space_sequencer/sequencer_strips_batch.cc
View File

@@ -8,7 +8,10 @@
#include "sequencer_strips_batch.hh"
#include "BLI_rect.h"
#include "DNA_userdef_types.h"
#include "DNA_view2d_types.h"
#include "GPU_batch.hh"
#include "GPU_batch_presets.hh"
@@ -39,12 +42,15 @@ float calc_strip_round_radius(float pixely)
return 8.0f;
}
StripsDrawBatch::StripsDrawBatch(float pixelx, float pixely) : strips_(GPU_SEQ_STRIP_DRAW_DATA_LEN)
StripsDrawBatch::StripsDrawBatch(const View2D *v2d) : strips_(GPU_SEQ_STRIP_DRAW_DATA_LEN)
{
context_.pixelx = pixelx;
context_.pixely = pixely;
context_.inv_pixelx = 1.0f / pixelx;
context_.inv_pixely = 1.0f / pixely;
view_mask_min_ = float2(v2d->mask.xmin, v2d->mask.ymin);
view_mask_size_ = float2(BLI_rcti_size_x(&v2d->mask), BLI_rcti_size_y(&v2d->mask));
view_cur_min_ = float2(v2d->cur.xmin, v2d->cur.ymin);
float2 view_cur_size = float2(BLI_rctf_size_x(&v2d->cur), BLI_rctf_size_y(&v2d->cur));
view_cur_inv_size_ = 1.0f / view_cur_size;
float pixely = view_cur_size.y / view_mask_size_.y;
context_.round_radius = calc_strip_round_radius(pixely);
context_.pixelsize = U.pixelsize;
@@ -91,14 +97,14 @@ SeqStripDrawData &StripsDrawBatch::add_strip(float content_start,
strips_count_++;
memset(&res, 0, sizeof(res));
res.content_start = content_start;
res.content_end = content_end;
res.top = top;
res.bottom = bottom;
res.strip_content_top = content_top;
res.left_handle = left_handle;
res.right_handle = right_handle;
res.handle_width = handle_width;
res.content_start = pos_to_pixel_space_x(content_start);
res.content_end = pos_to_pixel_space_x(content_end);
res.top = pos_to_pixel_space_y(top);
res.bottom = pos_to_pixel_space_y(bottom);
res.strip_content_top = pos_to_pixel_space_y(content_top);
res.left_handle = pos_to_pixel_space_x(left_handle);
res.right_handle = pos_to_pixel_space_x(right_handle);
res.handle_width = size_to_pixel_space_x(handle_width);
if (single_image) {
res.flags |= GPU_SEQ_FLAG_SINGLE_IMAGE;
}

30
source/blender/editors/space_sequencer/sequencer_strips_batch.hh
View File

@@ -9,10 +9,12 @@
#pragma once
#include "BLI_array.hh"
#include "BLI_math_vector_types.hh"
#include "GPU_shader_shared.hh"
struct GPUShader;
struct GPUUniformBuf;
struct View2D;
namespace blender::gpu {
class Batch;
@@ -20,6 +22,11 @@ class Batch;
namespace blender::ed::seq {
/* Utility to draw VSE timeline strip widgets in batches, with a dedicated
* shader. Internally, strip data for drawing is encoded into a uniform
* buffer. Strip coordinates are converted into pixel space, to avoid
* precision issues at large frames. Drawing assumes that a pixel space
* projection matrix is set. */
class StripsDrawBatch {
SeqContextDrawData context_;
Array<SeqStripDrawData> strips_;
@@ -31,8 +38,13 @@ class StripsDrawBatch {
int binding_strips_ = 0;
int strips_count_ = 0;
float2 view_mask_min_;
float2 view_mask_size_;
float2 view_cur_min_;
float2 view_cur_inv_size_;
public:
StripsDrawBatch(float pixelx, float pixely);
StripsDrawBatch(const View2D *v2d);
~StripsDrawBatch();
SeqStripDrawData &add_strip(float content_start,
@@ -46,6 +58,22 @@ class StripsDrawBatch {
bool single_image);
void flush_batch();
private:
/* Same math as `UI_view2d_view_to_region_*` but avoiding divisions,
* and without relying on View2D data type. */
inline float pos_to_pixel_space_x(float x) const
{
return (view_mask_min_.x + (x - view_cur_min_.x) * view_cur_inv_size_.x) * view_mask_size_.x;
}
inline float pos_to_pixel_space_y(float y) const
{
return (view_mask_min_.y + (y - view_cur_min_.y) * view_cur_inv_size_.y) * view_mask_size_.y;
}
inline float size_to_pixel_space_x(float x) const
{
return x * view_cur_inv_size_.x * view_mask_size_.x;
}
};
uint color_pack(const uchar rgba[4]);

9
source/blender/editors/space_sequencer/sequencer_timeline_draw.cc
View File

@@ -1253,6 +1253,9 @@ static void draw_strips_background(TimelineDrawContext *timeline_ctx,
StripsDrawBatch &strips_batch,
const Vector<StripDrawContext> &strips)
{
GPU_matrix_push_projection();
wmOrtho2_region_pixelspace(timeline_ctx->region);
GPU_blend(GPU_BLEND_ALPHA_PREMULT);
const bool show_overlay = (timeline_ctx->sseq->flag & SEQ_SHOW_OVERLAY) != 0;
@@ -1322,6 +1325,7 @@ static void draw_strips_background(TimelineDrawContext *timeline_ctx,
}
strips_batch.flush_batch();
GPU_blend(GPU_BLEND_ALPHA);
GPU_matrix_pop_projection();
}
static void strip_data_missing_media_flags_set(const StripDrawContext &strip,
@@ -1449,6 +1453,8 @@ static void draw_strips_foreground(TimelineDrawContext *timeline_ctx,
StripsDrawBatch &strips_batch,
const Vector<StripDrawContext> &strips)
{
GPU_matrix_push_projection();
wmOrtho2_region_pixelspace(timeline_ctx->region);
GPU_blend(GPU_BLEND_ALPHA_PREMULT);
for (const StripDrawContext &strip : strips) {
@@ -1471,6 +1477,7 @@ static void draw_strips_foreground(TimelineDrawContext *timeline_ctx,
strips_batch.flush_batch();
GPU_blend(GPU_BLEND_ALPHA);
GPU_matrix_pop_projection();
}
static void draw_retiming_continuity_ranges(TimelineDrawContext *timeline_ctx,
@@ -1908,7 +1915,7 @@ void draw_timeline_seq(const bContext *C, ARegion *region)
{
SeqQuadsBatch quads_batch;
TimelineDrawContext ctx = timeline_draw_context_get(C, &quads_batch);
StripsDrawBatch strips_batch(ctx.pixelx, ctx.pixely);
StripsDrawBatch strips_batch(ctx.v2d);
draw_timeline_pre_view_callbacks(&ctx);
UI_ThemeClearColor(TH_BACK);

2
source/blender/gpu/GPU_shader_shared.hh
View File

@@ -140,8 +140,6 @@ BLI_STATIC_ASSERT(sizeof(SeqStripDrawData) * GPU_SEQ_STRIP_DRAW_DATA_LEN <= 1638
/* VSE global data for timeline rendering. */
struct SeqContextDrawData {
float pixelx, pixely; /* Size of one pixel in timeline coordinate space. */
float inv_pixelx, inv_pixely;
float round_radius;
float pixelsize;
uint col_back;

17
source/blender/gpu/shaders/gpu_shader_sequencer_strips_frag.glsl
View File

@@ -44,18 +44,15 @@ void main()
SeqStripDrawData strip = strip_data[strip_id];
/* Transform strip rectangle into pixel coordinates, so that
* rounded corners have proper aspect ratio and can be expressed in pixels.
* Also snap to pixel grid coordinates, so that outline/border is clear
* non-fractional pixel sizes. */
vec2 view_to_pixel = vec2(context_data.inv_pixelx, context_data.inv_pixely);
vec2 pos1 = round(vec2(strip.left_handle, strip.bottom) * view_to_pixel);
vec2 pos2 = round(vec2(strip.right_handle, strip.top) * view_to_pixel);
/* Snap to pixel grid coordinates, so that outline/border is non-fractional
* pixel sizes. */
vec2 pos1 = round(vec2(strip.left_handle, strip.bottom));
vec2 pos2 = round(vec2(strip.right_handle, strip.top));
/* Make sure strip is at least 1px wide. */
pos2.x = max(pos2.x, pos1.x + 1.0);
vec2 size = (pos2 - pos1) * 0.5;
vec2 center = (pos1 + pos2) * 0.5;
vec2 pos = round(co * view_to_pixel);
vec2 pos = round(co);
float radius = context_data.round_radius;
if (radius > size.x) {
@@ -72,7 +69,7 @@ void main()
/* Distance to inner part when handles are taken into account. */
float sdf_inner = sdf;
if ((strip.flags & GPU_SEQ_FLAG_ANY_HANDLE) != 0) {
float handle_width = strip.handle_width * view_to_pixel.x;
float handle_width = strip.handle_width;
/* Take left/right handle from horizontal sides. */
if ((strip.flags & GPU_SEQ_FLAG_DRAW_LH) != 0) {
pos1.x += handle_width;
@@ -107,7 +104,7 @@ void main()
if (co.y < strip.strip_content_top) {
col.rgb = unpackUnorm4x8(strip.col_color_band).rgb;
/* Darker line to better separate the color band. */
if (co.y > strip.strip_content_top - context_data.pixely) {
if (co.y > strip.strip_content_top - 1.0) {
col.rgb = color_shade(col.rgb, -20.0);
}
}

4
source/blender/gpu/shaders/gpu_shader_sequencer_strips_vert.glsl
View File

@@ -9,8 +9,8 @@ void main()
int vid = gl_VertexID;
SeqStripDrawData strip = strip_data[id];
vec4 rect = vec4(strip.left_handle, strip.bottom, strip.right_handle, strip.top);
/* Expand by 2px to fit possible pixel grid rounding. */
vec2 expand = vec2(context_data.pixelx, context_data.pixely);
/* Expand by 1px to fit pixel grid rounding. */
vec2 expand = vec2(1.0, 1.0);
rect.xy -= expand;
rect.zw += expand;