blender
/
blender
136
407
Fork 589
Code Issues 6.3k Pull Requests 777 Projects 19 Wiki Activity

Image Editor Vectorscope Improvement #116974

Merged
Aras Pranckevicius merged 13 commits from JonasDichelle/blender:vectorscope-update into main 2024-02-06 12:22:57 +01:00
Conversation 22 Commits 13 Files Changed 5 +14 -18
4 changed files with 14 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 24b7efa648 - Show all commits

2
source/blender/blenkernel/intern/colortools.cc
View File

@ -1365,8 +1365,6 @@ static void save_sample_line(
scopes->vecscope_rgb[color_idx + 2] = rgb[2];
scopes->vecscope_rgb[color_idx + 3] = scopes->vecscope_alpha;
aras_p marked this conversation as resolved
Aras Pranckevicius commented 2024-02-01 16:03:40 +01:00
Review
Copy Link

Given that vecscope_rgb always 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 from vecscope_alpha field.

Given that `vecscope_rgb` always 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 from `vecscope_alpha` field.
Jonas Dichelle commented 2024-02-01 16:13:51 +01:00
Review
Copy Link

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.

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.
Aras Pranckevicius commented 2024-02-01 16:21:47 +01:00
Review
Copy Link

Ah, I see, this array is directly copied into the GPU vertex buffer. Ignore me then!

Ah, I see, this array is directly copied into the GPU vertex buffer. Ignore me then!
👍 1
/* Waveform. */
switch (scopes->wavefrm_mode) {
case SCOPES_WAVEFRM_RGB:

18
source/blender/editors/interface/interface_draw.cc
View File

@ -610,7 +610,6 @@ static void waveform_draw_one(float *waveform, int waveform_num, const float col
GPU_batch_discard(batch);
}
static void waveform_draw_rgb(float *waveform, int waveform_num, float *col)
{
GPUVertFormat format = {0};
@ -649,7 +648,6 @@ static void circle_draw_rgb(float *points, int tot_points, float *col, GPUPrimTy
GPU_batch_discard(batch);
}
void ui_draw_but_WAVEFORM(ARegion * /*region*/,
uiBut *but,
const uiWidgetColors * /*wcol*/,
@ -965,12 +963,12 @@ void ui_draw_but_VECTORSCOPE(ARegion * /*region*/,
Scopes *scopes = (Scopes *)but->poin;
const float colors[6][3] = {
{0.75, 0.0, 0.0}, /* Red */
{0.75, 0.75, 0.0}, /* Yellow */
{0.0, 0.75, 0.0}, /* Green */
{0.0, 0.75, 0.75}, /* Cyan */
{0.0, 0.0, 0.75}, /* Blue */
{0.75, 0.0, 0.75}, /* Magenta */
{0.75, 0.0, 0.0}, /* Red */
{0.75, 0.75, 0.0}, /* Yellow */
{0.0, 0.75, 0.0}, /* Green */
{0.0, 0.75, 0.75}, /* Cyan */
{0.0, 0.0, 0.75}, /* Blue */
{0.75, 0.0, 0.75}, /* Magenta */
};
const char color_names[] = {'R', 'Y', 'G', 'C', 'B', 'M'};
@ -1055,7 +1053,8 @@ void ui_draw_but_VECTORSCOPE(ARegion * /*region*/,
}
GPU_blend(GPU_BLEND_ALPHA);
circle_draw_rgb(circle_fill_points, tot_points+2, circle_fill_vertex_colors, GPU_PRIM_TRI_FAN);
circle_draw_rgb(
circle_fill_points, tot_points + 2, circle_fill_vertex_colors, GPU_PRIM_TRI_FAN);
}
/* draw filled Gray circle for background, only for RGB mode */
else if (scopes->vecscope_mode == SCOPES_VECSCOPE_RGB) {
@ -1122,7 +1121,6 @@ void ui_draw_but_VECTORSCOPE(ARegion * /*region*/,
circle_draw_rgb(inner_circle_points, tot_points, inner_circle_colors, GPU_PRIM_LINE_LOOP);
}
/* draw grid elements */
/* cross */
immUniformColor4f(1.0f, 1.0f, 1.0f, 0.1f);

4
source/blender/makesdna/DNA_color_types.h
View File

@ -180,8 +180,8 @@ enum {
/** #Scopes.vecscope_mode */
enum {
SCOPES_VECSCOPE_RGB = 0,
SCOPES_VECSCOPE_LUMA = 1,
SCOPES_VECSCOPE_RGB = 0,
SCOPES_VECSCOPE_LUMA = 1,
};
typedef struct ColorManagedViewSettings {

8
source/blender/makesrna/intern/rna_color.cc
View File

@ -1145,11 +1145,11 @@ static void rna_def_scopes(BlenderRNA *brna)
};
static const EnumPropertyItem prop_vecscope_mode_items[] = {
{SCOPES_VECSCOPE_LUMA, "LUMA", ICON_COLOR, "Luma", ""},
{SCOPES_VECSCOPE_RGB, "RGB", ICON_COLOR, "Red Green Blue", ""},
{0, nullptr, 0, nullptr, nullptr},
{SCOPES_VECSCOPE_LUMA, "LUMA", ICON_COLOR, "Luma", ""},
{SCOPES_VECSCOPE_RGB, "RGB", ICON_COLOR, "Red Green Blue", ""},
{0, nullptr, 0, nullptr, nullptr},
aras_p marked this conversation as resolved Outdated
Aras Pranckevicius commented 2024-02-01 16:04:22 +01:00
Outdated
Review
Copy Link

"Red Green Blue" as a UI label feels a bit weird (and hardly fits into UI control space). Maybe "RGB" or "Color" instead?

"Red Green Blue" as a UI label feels a bit weird (and hardly fits into UI control space). Maybe "RGB" or "Color" instead?
Pablo Vazquez commented 2024-02-01 16:06:50 +01:00
Outdated
Review
Copy Link

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 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).
👍 1
Jonas Dichelle commented 2024-02-01 16:08:39 +01:00
Outdated
Review
Copy Link

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.

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.
};
srna = RNA_def_struct(brna, "Scopes", nullptr);
RNA_def_struct_ui_text(srna, "Scopes", "Scopes for statistical view of an image");