blender
/
blender
136
406
Fork 589
Code Issues 6.3k Pull Requests 766 Projects 19 Wiki Activity

Viewport: draw light icons using the light colors #105236

Closed
Weizhen Huang wants to merge 5 commits from weizhen:colorful_light_icon into main
pull from: weizhen:colorful_light_icon
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 1 Commits 5 Files Changed 7 +85 -34
4 changed files with 85 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit aa4201dc7f - Show all commits

32
source/blender/draw/engines/overlay/overlay_extra.cc
View File

@ -123,7 +123,9 @@ void OVERLAY_extra_cache_init(OVERLAY_Data *vedata)
cb->field_tube_limit = BUF_INSTANCE(grp_sub, format, DRW_cache_field_tube_limit_get());
cb->field_vortex = BUF_INSTANCE(grp_sub, format, DRW_cache_field_vortex_get());
cb->field_wind = BUF_INSTANCE(grp_sub, format, DRW_cache_field_wind_get());
cb->light_icon = BUF_INSTANCE(grp_sub, format, DRW_cache_light_icon_lines_get());
cb->light_icon_inner = BUF_INSTANCE(grp_sub, format, DRW_cache_light_icon_inner_lines_get());
cb->light_icon_outer = BUF_INSTANCE(grp_sub, format, DRW_cache_light_icon_outer_lines_get());
cb->light_icon_sun_rays = BUF_INSTANCE(grp_sub, format, DRW_cache_light_icon_sun_rays_get());
cb->light_area[0] = BUF_INSTANCE(grp_sub, format, DRW_cache_light_area_disk_lines_get());
cb->light_area[1] = BUF_INSTANCE(grp_sub, format, DRW_cache_light_area_square_lines_get());
cb->light_point = BUF_INSTANCE(grp_sub, format, DRW_cache_light_point_lines_get());
@ -608,7 +610,7 @@ void OVERLAY_light_cache_populate(OVERLAY_Data *vedata, Object *ob)
DRW_object_wire_theme_get(ob, view_layer, &color_p);
/* Remove the alpha. */
float color[4] = {UNPACK3(color_p), 1.0f};
float theme_color[4] = {UNPACK3(color_p), 1.0f};
/* Pack render data into object matrix. */
union {
float mat[4][4];
@ -638,23 +640,25 @@ void OVERLAY_light_cache_populate(OVERLAY_Data *vedata, Object *ob)
DRW_buffer_add_entry(cb->groundline, instdata.pos);
float color_icon[4] = {1.0f};
float light_color[4] = {1.0f};
const bool show_light_colors = vedata->stl->pd->overlay.flag & V3D_OVERLAY_SHOW_LIGHT_COLORS;
if (show_light_colors) {
/* Get light color. */
copy_v3_v3(color_icon, &la->r);
copy_v3_v3(light_color, &la->r);
Clément Foucault commented 2023-02-28 17:08:05 +01:00
Review
Copy Link

I am unsure about using the light color as is. I think it should be normalized.

For instance, a light with color (0.0, 0.01, 0.0) at really high power will still give a powerful green.

I am unsure about using the light color as is. I think it should be normalized. For instance, a light with color `(0.0, 0.01, 0.0)` at really high power will still give a powerful green.
Weizhen Huang commented 2023-02-28 17:34:02 +01:00
Review
Copy Link

Not sure about getting into such details, because different light types with the same power could have quite different perceptual strength, it also depends on the size/radius and the spread of the light, a lot of things need to be considered.. we could do this in another commit if needed.

Not sure about getting into such details, because different light types with the same power could have quite different perceptual strength, it also depends on the size/radius and the spread of the light, a lot of things need to be considered.. we could do this in another commit if needed.
Clément Foucault commented 2023-02-28 18:26:54 +01:00
Review
Copy Link

Maybe there is a misunderstanding: I'm not asking about taking light perceptual intensity into account, I'm saying that the color should be normalized as it is the "tint" we are interested in for display. Having a nearly black light overlay makes zero sense to me.

I'm just asking to replace copy_v3_v3(light_color, &la->r); by normalize_v3_v3(light_color, &la->r);.

Maybe there is a misunderstanding: I'm not asking about taking light perceptual intensity into account, I'm saying that the color should be normalized as it is the "tint" we are interested in for display. Having a nearly black light overlay makes zero sense to me. I'm just asking to replace `copy_v3_v3(light_color, &la->r);` by `normalize_v3_v3(light_color, &la->r);`.
Andy Goralczyk commented 2023-03-07 14:48:33 +01:00
Review
Copy Link

For the record: Today I discussed this with @weizhen and @fclem and we decided to go with the non-normalized display for the time being. If many users struggle to find their lights, we can still look into normalization (or other ways to make the overlay more visible).

For the record: Today I discussed this with @weizhen and @fclem and we decided to go with the non-normalized display for the time being. If many users struggle to find their lights, we can still look into normalization (or other ways to make the overlay more visible).
👍 1
}
else {
copy_v4_v4(color_icon, color);
}
DRW_buffer_add_entry(cb->light_icon, color_icon, &instdata);
weizhen marked this conversation as resolved
Jeroen Bakker commented 2023-02-28 04:22:06 +01:00
Review
Copy Link

In drawing (where performance matters) we don't use RNA. You can just read the data directly from the la.

In drawing (where performance matters) we don't use RNA. You can just read the data directly from the `la`.
/* Draw the outer ring of the light icon and the sun rays in `light_color`, if required. */
DRW_buffer_add_entry(
cb->light_icon_outer, show_light_colors ? light_color : theme_color, &instdata);
DRW_buffer_add_entry(cb->light_icon_inner, theme_color, &instdata);
if (la->type == LA_LOCAL) {
instdata.area_size_x = instdata.area_size_y = la->radius;
DRW_buffer_add_entry(cb->light_point, color, &instdata);
DRW_buffer_add_entry(cb->light_point, theme_color, &instdata);
}
else if (la->type == LA_SUN) {
DRW_buffer_add_entry(cb->light_sun, color, &instdata);
DRW_buffer_add_entry(cb->light_sun, theme_color, &instdata);
DRW_buffer_add_entry(
cb->light_icon_sun_rays, show_light_colors ? light_color : theme_color, &instdata);
}
else if (la->type == LA_SPOT) {
/* Previous implementation was using the clip-end distance as cone size.
@ -677,8 +681,8 @@ void OVERLAY_light_cache_populate(OVERLAY_Data *vedata, Object *ob)
instdata.spot_blend = sqrtf((a2 - a2 * c2) / (c2 - a2 * c2));
instdata.spot_cosine = a;
/* HACK: We pack the area size in alpha color. This is decoded by the shader. */
color[3] = -max_ff(la->radius, FLT_MIN);
DRW_buffer_add_entry(cb->light_spot, color, &instdata);
theme_color[3] = -max_ff(la->radius, FLT_MIN);
DRW_buffer_add_entry(cb->light_spot, theme_color, &instdata);
if ((la->mode & LA_SHOW_CONE) && !DRW_state_is_select()) {
const float color_inside[4] = {0.0f, 0.0f, 0.0f, 0.5f};
@ -692,7 +696,7 @@ void OVERLAY_light_cache_populate(OVERLAY_Data *vedata, Object *ob)
int sqr = ELEM(la->area_shape, LA_AREA_SQUARE, LA_AREA_RECT);
instdata.area_size_x = la->area_size;
instdata.area_size_y = uniform_scale ? la->area_size : la->area_sizey;
DRW_buffer_add_entry(cb->light_area[sqr], color, &instdata);
DRW_buffer_add_entry(cb->light_area[sqr], theme_color, &instdata);
}
}

4
source/blender/draw/engines/overlay/overlay_private.hh
View File

@ -178,7 +178,9 @@ typedef struct OVERLAY_ExtraCallBuffers {
DRWCallBuffer *groundline;
DRWCallBuffer *light_icon;
DRWCallBuffer *light_icon_inner;
Jeroen Bakker commented 2023-02-28 04:30:19 +01:00
Review
Copy Link

I know this is just a WIP ticket and you can apply these changes whenever it's appropriate.

perhaps light_center should be a better description. (also for the other related methods).

I know this is just a WIP ticket and you can apply these changes whenever it's appropriate. perhaps `light_center` should be a better description. (also for the other related methods).
Weizhen Huang commented 2023-02-28 16:36:08 +01:00
Review
Copy Link

I am following the comment I deleted for drawing the part, it was /* Light Icon */. Now we draw the inner circles and the outer circle separately, it's weird to call all of them "center". If "icon" is not a good name, I could probably change them to DRW_cache_light_inner_circles_get(), DRW_cache_light_outer_circle_get()? Then I will also adjust it in the commit message.

I am following the comment I deleted for drawing the part, it was `/* Light Icon */`. Now we draw the inner circles and the outer circle separately, it's weird to call all of them "center". If "icon" is not a good name, I could probably change them to `DRW_cache_light_inner_circles_get()`, `DRW_cache_light_outer_circle_get()`? Then I will also adjust it in the commit message.
DRWCallBuffer *light_icon_outer;
DRWCallBuffer *light_icon_sun_rays;
DRWCallBuffer *light_point;
DRWCallBuffer *light_sun;
DRWCallBuffer *light_spot;

79
source/blender/draw/intern/draw_cache.c
View File

@ -116,7 +116,9 @@ static struct DRWShapeCache {
GPUBatch *drw_field_cone_limit;
GPUBatch *drw_field_sphere_limit;
GPUBatch *drw_ground_line;
GPUBatch *drw_light_icon_lines;
GPUBatch *drw_light_icon_inner_lines;
GPUBatch *drw_light_icon_outer_lines;
GPUBatch *drw_light_icon_sun_rays;
GPUBatch *drw_light_point_lines;
GPUBatch *drw_light_sun_lines;
GPUBatch *drw_light_spot_lines;
@ -1462,12 +1464,12 @@ GPUBatch *DRW_cache_groundline_get(void)
return SHC.drw_ground_line;
}
GPUBatch *DRW_cache_light_icon_lines_get(void)
GPUBatch *DRW_cache_light_icon_inner_lines_get(void)
{
if (!SHC.drw_light_icon_lines) {
if (!SHC.drw_light_icon_inner_lines) {
GPUVertFormat format = extra_vert_format();
int v_len = 2 * (DIAMOND_NSEGMENTS + INNER_NSEGMENTS + OUTER_NSEGMENTS);
int v_len = 2 * (DIAMOND_NSEGMENTS + INNER_NSEGMENTS);
GPUVertBuf *vbo = GPU_vertbuf_create_with_format(&format);
GPU_vertbuf_data_alloc(vbo, v_len);
@ -1476,11 +1478,63 @@ GPUBatch *DRW_cache_light_icon_lines_get(void)
circle_verts(vbo, &v, DIAMOND_NSEGMENTS, r * 0.3f, 0.0f, VCLASS_SCREENSPACE);
circle_dashed_verts(vbo, &v, INNER_NSEGMENTS, r * 1.0f, 0.0f, VCLASS_SCREENSPACE);
SHC.drw_light_icon_inner_lines = GPU_batch_create_ex(
GPU_PRIM_LINES, vbo, NULL, GPU_BATCH_OWNS_VBO);
}
return SHC.drw_light_icon_inner_lines;
}
GPUBatch *DRW_cache_light_icon_outer_lines_get(void)
{
if (!SHC.drw_light_icon_outer_lines) {
GPUVertFormat format = extra_vert_format();
int v_len = 2 * OUTER_NSEGMENTS;
GPUVertBuf *vbo = GPU_vertbuf_create_with_format(&format);
GPU_vertbuf_data_alloc(vbo, v_len);
const float r = 9.0f;
int v = 0;
circle_dashed_verts(vbo, &v, OUTER_NSEGMENTS, r * 1.33f, 0.0f, VCLASS_SCREENSPACE);
SHC.drw_light_icon_lines = GPU_batch_create_ex(GPU_PRIM_LINES, vbo, NULL, GPU_BATCH_OWNS_VBO);
SHC.drw_light_icon_outer_lines = GPU_batch_create_ex(
GPU_PRIM_LINES, vbo, NULL, GPU_BATCH_OWNS_VBO);
}
return SHC.drw_light_icon_lines;
return SHC.drw_light_icon_outer_lines;
}
GPUBatch *DRW_cache_light_icon_sun_rays_get(void)
{
if (!SHC.drw_light_icon_sun_rays) {
GPUVertFormat format = extra_vert_format();
const int num_rays = 8;
int v_len = 4 * num_rays;
GPUVertBuf *vbo = GPU_vertbuf_create_with_format(&format);
GPU_vertbuf_data_alloc(vbo, v_len);
const float r = 9.0f;
int v = 0;
/* Sun Rays */
for (int a = 0; a < num_rays; a++) {
float angle = (2.0f * M_PI * a) / (float)num_rays;
float s = sinf(angle) * r;
float c = cosf(angle) * r;
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 1.6f, c * 1.6f, 0.0f}, VCLASS_SCREENSPACE});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 1.9f, c * 1.9f, 0.0f}, VCLASS_SCREENSPACE});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 2.2f, c * 2.2f, 0.0f}, VCLASS_SCREENSPACE});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 2.5f, c * 2.5f, 0.0f}, VCLASS_SCREENSPACE});
}
SHC.drw_light_icon_sun_rays = GPU_batch_create_ex(
GPU_PRIM_LINES, vbo, NULL, GPU_BATCH_OWNS_VBO);
}
return SHC.drw_light_icon_sun_rays;
}
GPUBatch *DRW_cache_light_point_lines_get(void)
@ -1508,23 +1562,12 @@ GPUBatch *DRW_cache_light_sun_lines_get(void)
if (!SHC.drw_light_sun_lines) {
GPUVertFormat format = extra_vert_format();
int v_len = 2 * (8 * 2 + 1);
int v_len = 2;
GPUVertBuf *vbo = GPU_vertbuf_create_with_format(&format);
GPU_vertbuf_data_alloc(vbo, v_len);
const float r = 9.0f;
int v = 0;
/* Sun Rays */
for (int a = 0; a < 8; a++) {
float angle = (2.0f * M_PI * a) / 8.0f;
float s = sinf(angle) * r;
float c = cosf(angle) * r;
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 1.6f, c * 1.6f, 0.0f}, VCLASS_SCREENSPACE});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 1.9f, c * 1.9f, 0.0f}, VCLASS_SCREENSPACE});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 2.2f, c * 2.2f, 0.0f}, VCLASS_SCREENSPACE});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{s * 2.5f, c * 2.5f, 0.0f}, VCLASS_SCREENSPACE});
}
/* Direction Line */
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{0.0, 0.0, 0.0}, 0});
GPU_vertbuf_vert_set(vbo, v++, &(Vert){{0.0, 0.0, -20.0}, 0}); /* Good default. */

4
source/blender/draw/intern/draw_cache.h
View File

@ -106,7 +106,9 @@ struct GPUBatch *DRW_cache_field_sphere_limit_get(void);
/* Lights */
struct GPUBatch *DRW_cache_light_icon_lines_get(void);
struct GPUBatch *DRW_cache_light_icon_inner_lines_get(void);
struct GPUBatch *DRW_cache_light_icon_outer_lines_get(void);
struct GPUBatch *DRW_cache_light_icon_sun_rays_get(void);
struct GPUBatch *DRW_cache_light_point_lines_get(void);
struct GPUBatch *DRW_cache_light_sun_lines_get(void);
struct GPUBatch *DRW_cache_light_spot_lines_get(void);