Fix #111017: Gap between color square and frame of box in the color ramp #111168

Philipp Oeser · 2023-08-16T09:59:05+02:00

Philipp Oeser commented

2023-08-16 09:59:05 +02:00

While not a real fix, I tracked the reason down to
ui_draw_colorband_handle_box(in the non-filled case) somewhat
misbehaving:

it is not the same size as the filled one
-- [1] top line is shifted a pixel up
-- [2] right line is shifter a pixel out
-- [3] top right corner pixel missing

But it even acts wrong if you substitute ui_draw_colorband_handle_box
with imm_draw_box_wire_2d.
Both only act wrong in the context of the colorband though (and are
working fine elsewhere) which is a bit weired and I could not track it
down. Of course we could counter-act by adjusting the width and height
that is passed to ui_draw_colorband_handle_box, but that still does
not solve the corner pixel missing.

In this case we can use a workaround though: just draw the box filled.

While not a real fix, I tracked the reason down to `ui_draw_colorband_handle_box`(in the non-filled case) somewhat misbehaving: - it is not the same size as the filled one -- [1] top line is shifted a pixel up -- [2] right line is shifter a pixel out -- [3] top right corner pixel missing But it even acts wrong if you substitute `ui_draw_colorband_handle_box` with `imm_draw_box_wire_2d`. Both only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weired and I could not track it down. Of course we could counter-act by adjusting the width and height that is passed to `ui_draw_colorband_handle_box`, but that still does not solve the corner pixel missing. In this case we can use a workaround though: just draw the box filled.

Before.png

24 KiB

After.png

8.9 KiB

👍 1 ❤️ 2

Philipp Oeser added 1 commit 2023-08-16 09:59:16 +02:00

45f26d05c5 Fix #111017 : Gap between color square and frame of box in the color ramp

While not a real fix, I tracked the reason down to
`ui_draw_colorband_handle_box`(in the non-filled case) somewhat
misbehaving:
- it is not the same size as the filled one
-- [1] top line is shifted a pixel up
-- [2] right line is shifter a pixel out
-- [3] top right corner pixel missing

But it even acts wrong if you substitute `ui_draw_colorband_handle_box`
with `imm_draw_box_wire_2d`.
Both only act wrong in the context of the colorband though (and are
working fine elsewhere) which is a bit weired and I could not track it
down. Of course we could counter-act by adjusting the width and height
that is passed to `ui_draw_colorband_handle_box`, but that still does
not solve the corner pixel missing.

In this case we can use a workaround though: just draw the box filled.

Philipp Oeser added this to the User Interface project 2023-08-16 10:00:52 +02:00

Philipp Oeser requested review from Harley Acheson 2023-08-16 10:01:02 +02:00

Philipp Oeser requested review from Pablo Vazquez 2023-08-16 10:01:11 +02:00

Pablo Vazquez approved these changes 2023-08-16 19:14:46 +02:00

Pablo Vazquez left a comment

Nice little tweak! +1 from the design point of view.

YimingWu commented

2023-08-19 10:59:45 +02:00

Huh interesting... I just -1'd the black outline width... This is probably due to half pixel aligning. If visually passable it's gonna be alright.

  immUniformColor3ub(0, 0, 0);
  ui_draw_colorband_handle_box(
      shdr_pos, x - half_width, y1 - 1, x + half_width - 1, y1 + height, false);

Huh interesting... I just `-1`'d the black outline width... This is probably due to half pixel aligning. If visually passable it's gonna be alright. ``` immUniformColor3ub(0, 0, 0); ui_draw_colorband_handle_box( shdr_pos, x - half_width, y1 - 1, x + half_width - 1, y1 + height, false); ``` ![图片](/attachments/fbb8236f-207b-408e-b72c-e77c9527ea03)

图片.png

1.8 KiB

Philipp Oeser commented

2023-08-21 12:38:32 +02:00

Huh interesting... I just -1'd the black outline width... This is probably due to half pixel aligning. If visually passable it's gonna be alright.
  immUniformColor3ub(0, 0, 0);
  ui_draw_colorband_handle_box(
      shdr_pos, x - half_width, y1 - 1, x + half_width - 1, y1 + height, false);

No strong opinion here (if it works reliably -- does it? would be strange if it works for some widths but fails for others...).
If we go this route, it should probably have a comment explaining the "one off" otherwise might be confusion and we risk someone changes is back later...)

> Huh interesting... I just `-1`'d the black outline width... This is probably due to half pixel aligning. If visually passable it's gonna be alright. > > ``` > immUniformColor3ub(0, 0, 0); > ui_draw_colorband_handle_box( > shdr_pos, x - half_width, y1 - 1, x + half_width - 1, y1 + height, false); > ``` > > ![图片](/attachments/fbb8236f-207b-408e-b72c-e77c9527ea03) No strong opinion here (if it works reliably -- does it? would be strange if it works for some widths but fails for others...). If we go this route, it should probably have a comment explaining the "one off" otherwise might be confusion and we risk someone changes is back later...)

YimingWu commented

2023-08-22 03:24:19 +02:00

would be strange if it works for some widths but fails for others...

Interesting... I haven't really tried but from the look of it, it seems all sizes have that extra 1 px that I don't think I fully understand the cause 🤔

> would be strange if it works for some widths but fails for others... Interesting... I haven't really tried but from the look of it, it seems all sizes have that extra 1 px that I don't think I fully understand the cause 🤔

Charlie Jolly commented

2023-08-22 04:32:58 +02:00

Originally the extra pixel was used to align the arrow with the color ramp line. Additionally all these widgets suffer from not having a clickable area outside the main ramp, but I guess that's another story! 👍

YimingWu commented

2023-08-22 04:36:32 +02:00

@CharlieJolly But the line seems to be 1 pixel to the right of the center of the arrow with or without the 1px added...

Charlie Jolly commented

2023-08-22 04:39:10 +02:00

@CharlieJolly But the line seems to be 1 pixel to the right of the center of the arrow with or without the 1px added...

This was one of my very first contributions to Blender so I have a vague recollection that the arrow and line used to line up.

I'm pretty sure this broke over time... probably from a drawing code refactor... hehe

> @CharlieJolly But the line seems to be 1 pixel to the right of the center of the arrow with or without the 1px added... This was one of my very first contributions to Blender so I have a vague recollection that the arrow and line used to line up. I'm pretty sure this broke over time... probably from a drawing code refactor... hehe

👍 1 😆 1

YimingWu commented

2023-08-22 04:45:57 +02:00

@CharlieJolly I think what we could do is having a 2 px line so it always line up :P

👀 1 😆 1

Charlie Jolly commented

2023-08-22 04:47:37 +02:00

@CharlieJolly I think what we could do is having a 2 px line so it always line up :P

Worth a try! Lol

> @CharlieJolly I think what we could do is having a 2 px line so it always line up :P Worth a try! Lol

Philipp Oeser commented

2023-08-22 09:09:01 +02:00

Additionally all these widgets suffer from not having a clickable area outside the main ramp, but I guess that's another story! 👍

That is reported in #103477

> Additionally all these widgets suffer from not having a clickable area outside the main ramp, but I guess that's another story! 👍 That is reported in #103477

👍 1

Philipp Oeser commented

2023-08-22 09:12:54 +02:00

Still think we should not rely on the non-filled version (if it can be unreliable in regards to actual widths -- and also having this missing pixel in the top-right corner)

👍 1

Charlie Jolly commented

2023-08-25 01:53:18 +02:00

Just for sentimental reasons, the old color ramp.

Just for sentimental reasons, the old color ramp. ![image](/attachments/3315d9e4-4454-4ea1-9f6d-219fe01c80ba)

image.png

30 KiB

🎉 1

Harley Acheson commented

2023-08-26 01:51:15 +02:00

@lichtwerk

When I look at these little things, in current master not your PR, I see them working or not, depending on the "Line Width"

If my Resolution Scale is set to 1.0X, then it shows the gap on the right when using default line width, but looks okay when set to "Thick" lines:

With my Resolution Scale set to 2.0X then it looks funny with Thin lines, looks fine at Default, and is a bit thick on the right when "Thick" lines:

So my guess is this will be related to U.pixelsize. But note that for regular GL lines many Macs can't make simple lines wider than a pixel. Which could make this complicated.

My gut feeling is that a way to fix this nicely might be to construct this so that it always uses single-pixel width lines and so do not get wider/narrower with line width. The other components, the triangle and the rectangle of color, could use offsets (like border around the color square) that respond to line width (U.pixelsize).

@lichtwerk When I look at these little things, in current master not your PR, I see them working or not, depending on the "Line Width" If my Resolution Scale is set to 1.0X, then it shows the gap on the right when using default line width, but looks okay when set to "Thick" lines: ![House1x.gif](/attachments/618a13c4-b323-41e3-bc3d-5c3fbc667310) With my Resolution Scale set to 2.0X then it looks funny with Thin lines, looks fine at Default, and is a bit thick on the right when "Thick" lines: ![House2x.gif](/attachments/ef36ec0d-96e5-4d5a-a66e-f68cf8452c74) So my guess is this will be related to U.pixelsize. But note that for regular GL lines many Macs can't make simple lines wider than a pixel. Which could make this complicated. My gut feeling is that a way to fix this nicely might be to construct this so that it always uses single-pixel width lines and so do not get wider/narrower with line width. The other components, the triangle and the rectangle of color, could use offsets (like border around the color square) that respond to line width (U.pixelsize).

House2x.gif

61 KiB

House1x.gif

15 KiB

Harley Acheson commented

2023-08-28 22:33:56 +02:00

I might be not be seeing all the oddness with my old eyes...

This indicator consists of a square with a triangle on top. The square itself is drawn as a black outline, a grey square, and a smaller square inside with the color. The black outline should be the same size as what it is outlining, but it is too big. Fairly certain the outline drawn with (0,0,0) should be at the exact position and same size as the grey box drawn with (128,128,128).

So I see most of the oddness going away with:

diff --git a/source/blender/editors/interface/interface_draw.cc b/source/blender/editors/interface/interface_draw.cc
index 8a377c83772..70679597958 100644
--- a/source/blender/editors/interface/interface_draw.cc
+++ b/source/blender/editors/interface/interface_draw.cc
@@ -1128,8 +1128,7 @@ static void ui_draw_colorband_handle(uint shdr_pos,
   y1 -= half_width;
 
   immUniformColor3ub(0, 0, 0);
-  ui_draw_colorband_handle_box(
-      shdr_pos, x - half_width, y1 - 1, x + half_width, y1 + height, false);
+  ui_draw_colorband_handle_box(shdr_pos, x - half_width, y1, x + half_width, y1 + height, false);
 
   /* draw all triangles blended */
   GPU_blend(GPU_BLEND_ALPHA);
@@ -1159,8 +1158,7 @@ static void ui_draw_colorband_handle(uint shdr_pos,
   GPU_blend(GPU_BLEND_NONE);
 
   immUniformColor3ub(128, 128, 128);
-  ui_draw_colorband_handle_box(
-      shdr_pos, x - (half_width - 1), y1, x + (half_width - 1), y1 + height, true);
+  ui_draw_colorband_handle_box(shdr_pos, x - half_width, y1, x + half_width, y1 + height, true);
 
   if (display) {
     IMB_colormanagement_scene_linear_to_display_v3(colf, display);
@@ -1168,7 +1166,7 @@ static void ui_draw_colorband_handle(uint shdr_pos,
 
   immUniformColor3fv(colf);
   ui_draw_colorband_handle_box(
-      shdr_pos, x - (half_width - 2), y1 + 1, x + (half_width - 2), y1 + height - 2, true);
+      shdr_pos, x - (half_width - 1), y1 + 1, x + (half_width - 1), y1 + height - 2, true);
 }
 
 void ui_draw_but_COLORBAND(uiBut *but, const uiWidgetColors * /*wcol*/, const rcti *rect)

I might be not be seeing all the oddness with my old eyes... This indicator consists of a square with a triangle on top. The square itself is drawn as a black outline, a grey square, and a smaller square inside with the color. The black outline should be the same size as what it is outlining, but it is too big. Fairly certain the outline drawn with (0,0,0) should be at the exact position and same size as the grey box drawn with (128,128,128). So I see most of the oddness going away with: ``` diff --git a/source/blender/editors/interface/interface_draw.cc b/source/blender/editors/interface/interface_draw.cc index 8a377c83772..70679597958 100644 --- a/source/blender/editors/interface/interface_draw.cc +++ b/source/blender/editors/interface/interface_draw.cc @@ -1128,8 +1128,7 @@ static void ui_draw_colorband_handle(uint shdr_pos, y1 -= half_width; immUniformColor3ub(0, 0, 0); - ui_draw_colorband_handle_box( - shdr_pos, x - half_width, y1 - 1, x + half_width, y1 + height, false); + ui_draw_colorband_handle_box(shdr_pos, x - half_width, y1, x + half_width, y1 + height, false); /* draw all triangles blended */ GPU_blend(GPU_BLEND_ALPHA); @@ -1159,8 +1158,7 @@ static void ui_draw_colorband_handle(uint shdr_pos, GPU_blend(GPU_BLEND_NONE); immUniformColor3ub(128, 128, 128); - ui_draw_colorband_handle_box( - shdr_pos, x - (half_width - 1), y1, x + (half_width - 1), y1 + height, true); + ui_draw_colorband_handle_box(shdr_pos, x - half_width, y1, x + half_width, y1 + height, true); if (display) { IMB_colormanagement_scene_linear_to_display_v3(colf, display); @@ -1168,7 +1166,7 @@ static void ui_draw_colorband_handle(uint shdr_pos, immUniformColor3fv(colf); ui_draw_colorband_handle_box( - shdr_pos, x - (half_width - 2), y1 + 1, x + (half_width - 2), y1 + height - 2, true); + shdr_pos, x - (half_width - 1), y1 + 1, x + (half_width - 1), y1 + height - 2, true); } void ui_draw_but_COLORBAND(uiBut *but, const uiWidgetColors * /*wcol*/, const rcti *rect) ``` ![image](/attachments/053dcf5e-5af2-4901-b2bb-87aea4f2f015)

image.png

7.0 KiB

❤️ 1

Philipp Oeser commented

2023-08-29 14:06:24 +02:00

Thx for all the experiments.

Still, it seems to me that:

fiddeling with the line widths and the positioning/coordinates fed to ui_draw_colorband_handle_box just to get it to draw right (instead of using coordintates that would seem "logical") is intransparent / error prone (at least without further comments)
but more importantly: is still draws wrong (notice the missing corner pixels):

Thx for all the experiments. Still, it seems to me that: - fiddeling with the line widths and the positioning/coordinates fed to ui_draw_colorband_handle_box just to get it to draw right (instead of using coordintates that would seem "logical") is intransparent / error prone (at least without further comments) - but more importantly: is still draws wrong (notice the missing corner pixels): ![image](/attachments/5dcef005-d470-466e-8675-2d6e4b6969f7)

image.png

5.0 KiB

Harley Acheson commented

2023-08-29 23:54:29 +02:00

but more importantly: is still draws wrong (notice the missing corner pixels):

Sadly, this is currently how all of our line drawing routines work for lines wider than a single pixel.

As in you can take any of our line drawing shaders, make a box, and if the line width is greater than a pixel it will be missing the pixels you point out. Luckily we don't do this often though so this isn't usually noticed.

And if you take those same positions and make a solid box instead it will do so with two solid triangles and it will look perfect.

The background to this is fairly interesting.

If you go back a while we just told OpenGL to make a line of 3 pixels wide or whatever, draw those lines and it will look fine without those missing corner bits. But... creating lines wider than a single pixel is an optional part of the standard and we had/have many Macs that do not make a line wider than a single pixel. Draw those on a Mac Retina display and those pixels are TINY.

So instead we have new shaders that create geometry as they draw. So we draw a single line but the shader expands that out to make wider lines. But our shader just expands out perpendicularly so we get problems at bending intersections. You can make Grease Pencil lines super wide and notice that there are gaps at tight bends.

The best solution to this particular problem if you want it perfect is to not use lines at all but only solid shapes. So the outer box outline is instead drawn as a black box, then the grey box inset by U.pixelsize, then the color box offset again.

> - but more importantly: is still draws wrong (notice the missing corner pixels): Sadly, this is currently how all of our line drawing routines work for lines wider than a single pixel. As in you can take any of our **line** drawing shaders, make a box, and if the line width is greater than a pixel it will be missing the pixels you point out. Luckily we don't do this often though so this isn't usually noticed. And if you take those same positions and make a solid box instead it will do so with two solid triangles and it will look perfect. The background to this is fairly interesting. If you go back a while we just told OpenGL to make a line of 3 pixels wide or whatever, draw those lines and it will look fine without those missing corner bits. But... creating lines wider than a single pixel is an optional part of the standard and we had/have many Macs that do not make a line wider than a single pixel. Draw those on a Mac Retina display and those pixels are TINY. So instead we have new shaders that create geometry as they draw. So we draw a single line but the shader expands that out to make wider lines. But our shader just expands out perpendicularly so we get problems at bending intersections. You can make Grease Pencil lines super wide and notice that there are gaps at tight bends. The best solution to this particular problem if you want it perfect is to not use lines at all but only solid shapes. So the outer box outline is instead drawn as a black box, then the grey box inset by U.pixelsize, then the color box offset again.

👀 1

Philipp Oeser commented

2023-08-30 05:42:19 +02:00

The best solution to this particular problem if you want it perfect is to not use lines at all but only solid shapes.

Tada! Exactly what the patch is doing, no?

> The best solution to this particular problem if you want it perfect is to not use lines at all but only solid shapes. Tada! Exactly what the patch is doing, no?

Harley Acheson commented

2023-08-31 03:50:37 +02:00

Tada! Exactly what the patch is doing, no?

For sure. But the same issue happens with the triangles, which also have to be drawn filled. And I am trying to get "Line Width" working as expected. With the following I can get it like the following (Thin, Default, Thick):

diff --git a/source/blender/editors/interface/interface_draw.cc b/source/blender/editors/interface/interface_draw.cc
index 8a377c83772..ca47cfcd235 100644
--- a/source/blender/editors/interface/interface_draw.cc
+++ b/source/blender/editors/interface/interface_draw.cc
@@ -1005,78 +1005,38 @@ void ui_draw_but_VECTORSCOPE(ARegion * /*region*/,
   immUnbindProgram();
 
   /* Restore scissor test. */
   GPU_scissor(UNPACK4(scissor));
   /* outline */
   draw_scope_end(&rect);
 
   GPU_blend(GPU_BLEND_NONE);
 }
 
-static void ui_draw_colorband_handle_tri_hlight(
+static void ui_draw_colorband_handle_tri(
     uint pos, float x1, float y1, float halfwidth, float height)
 {
-  GPU_line_smooth(true);
-
-  immBegin(GPU_PRIM_LINE_STRIP, 3);
+  immBegin(GPU_PRIM_TRIS, 3);
   immVertex2f(pos, x1 + halfwidth, y1);
   immVertex2f(pos, x1, y1 + height);
   immVertex2f(pos, x1 - halfwidth, y1);
   immEnd();
-
-  GPU_line_smooth(false);
 }
 
-static void ui_draw_colorband_handle_tri(
-    uint pos, float x1, float y1, float halfwidth, float height, bool fill)
+static void ui_draw_colorband_handle_box(uint pos, float x1, float y1, float x2, float y2)
 {
-  if (fill) {
-    GPU_polygon_smooth(true);
-  }
-  else {
-    GPU_line_smooth(true);
-  }
-
-  immBegin(fill ? GPU_PRIM_TRIS : GPU_PRIM_LINE_LOOP, 3);
-  immVertex2f(pos, x1 + halfwidth, y1);
-  immVertex2f(pos, x1, y1 + height);
-  immVertex2f(pos, x1 - halfwidth, y1);
+  immBegin(GPU_PRIM_TRI_STRIP, 4);
+  immVertex2f(pos, x2, y1);
+  immVertex2f(pos, x1, y1);
+  immVertex2f(pos, x2, y2);
+  immVertex2f(pos, x1, y2);
   immEnd();
-
-  if (fill) {
-    GPU_polygon_smooth(false);
-  }
-  else {
-    GPU_line_smooth(false);
-  }
-}
-
-static void ui_draw_colorband_handle_box(
-    uint pos, float x1, float y1, float x2, float y2, bool fill)
-{
-  if (fill) {
-    immBegin(GPU_PRIM_TRI_STRIP, 4);
-    immVertex2f(pos, x2, y1);
-    immVertex2f(pos, x1, y1);
-    immVertex2f(pos, x2, y2);
-    immVertex2f(pos, x1, y2);
-    immEnd();
-  }
-  else {
-    immBegin(GPU_PRIM_LINE_STRIP, 5);
-    immVertex2f(pos, x1, y1);
-    immVertex2f(pos, x1, y2);
-    immVertex2f(pos, x2, y2);
-    immVertex2f(pos, x2, y1);
-    immVertex2f(pos, x1, y1);
-    immEnd();
-  }
 }
 
 static void ui_draw_colorband_handle(uint shdr_pos,
                                      const rcti *rect,
                                      float x,
                                      const float rgb[3],
                                      ColorManagedDisplay *display,
                                      bool active)
 {
   const float sizey = BLI_rcti_size_y(rect);
@@ -1120,62 +1080,68 @@ static void ui_draw_colorband_handle(uint shdr_pos,
 
     /* hide handles when zoomed out too far */
     if (half_width < min_width) {
       return;
     }
   }
 
   /* shift handle down */
   y1 -= half_width;
 
+  /* Black outline around the lower box. */
   immUniformColor3ub(0, 0, 0);
-  ui_draw_colorband_handle_box(
-      shdr_pos, x - half_width, y1 - 1, x + half_width, y1 + height, false);
+  ui_draw_colorband_handle_box(shdr_pos,
+                               x - half_width - U.pixelsize,
+                               y1 - U.pixelsize,
+                               x + half_width + U.pixelsize,
+                               y1 + height);
+
+  /* Grey box inset by line width. */
+  immUniformColor3ub(128, 128, 128);
+  ui_draw_colorband_handle_box(shdr_pos, x - half_width, y1, x + half_width, y1 + height);
 
-  /* draw all triangles blended */
+  if (display) {
+    IMB_colormanagement_scene_linear_to_display_v3(colf, display);
+  }
+
+  /* Color value inset by another line width. */
+  immUniformColor3fv(colf);
+  ui_draw_colorband_handle_box(shdr_pos,
+                               x - (half_width - U.pixelsize),
+                               y1 + U.pixelsize,
+                               x + (half_width - U.pixelsize),
+                               y1 + height - U.pixelsize);
+
+  /* draw all triangles blended and smooth. */
   GPU_blend(GPU_BLEND_ALPHA);
+  GPU_polygon_smooth(true);
 
-  ui_draw_colorband_handle_tri(shdr_pos, x, y1 + height, half_width, half_width, true);
+  /* Black outline around the top triangle. */
+  immUniformColor3ub(0, 0, 0);
+  ui_draw_colorband_handle_tri(
+      shdr_pos, x, y1 + height, half_width + U.pixelsize, half_width + U.pixelsize);
 
+  /* Triangle main color. */
   if (active) {
     immUniformColor3ub(196, 196, 196);
   }
   else {
     immUniformColor3ub(96, 96, 96);
   }
-  ui_draw_colorband_handle_tri(shdr_pos, x, y1 + height, half_width, half_width, true);
-
-  if (active) {
-    immUniformColor3ub(255, 255, 255);
-  }
-  else {
-    immUniformColor3ub(128, 128, 128);
-  }
-  ui_draw_colorband_handle_tri_hlight(
-      shdr_pos, x, y1 + height - 1, (half_width - 1), (half_width - 1));
-
-  immUniformColor3ub(0, 0, 0);
-  ui_draw_colorband_handle_tri_hlight(shdr_pos, x, y1 + height, half_width, half_width);
+  ui_draw_colorband_handle_tri(shdr_pos,
+                               x,
+                               y1 + height,
+                               half_width - (0.5f * U.pixelsize),
+                               half_width - (0.5f * U.pixelsize));
 
+  GPU_polygon_smooth(false);
   GPU_blend(GPU_BLEND_NONE);
-
-  immUniformColor3ub(128, 128, 128);
-  ui_draw_colorband_handle_box(
-      shdr_pos, x - (half_width - 1), y1, x + (half_width - 1), y1 + height, true);
-
-  if (display) {
-    IMB_colormanagement_scene_linear_to_display_v3(colf, display);
-  }
-
-  immUniformColor3fv(colf);
-  ui_draw_colorband_handle_box(
-      shdr_pos, x - (half_width - 2), y1 + 1, x + (half_width - 2), y1 + height - 2, true);
 }
 
 void ui_draw_but_COLORBAND(uiBut *but, const uiWidgetColors * /*wcol*/, const rcti *rect)
 {
   ColorManagedDisplay *display = ui_block_cm_display_get(but->block);
   uint pos_id, col_id;
 
   uiButColorBand *but_coba = (uiButColorBand *)but;
   ColorBand *coba = (but_coba->edit_coba == nullptr) ? (ColorBand *)but->poin :
                                                        but_coba->edit_coba;

> Tada! Exactly what the patch is doing, no? For sure. But the same issue happens with the triangles, which also have to be drawn filled. And I am trying to get "Line Width" working as expected. With the following I can get it like the following (Thin, Default, Thick): ![image](/attachments/942300fe-54c1-4644-aefd-b82ada797758) ``` diff --git a/source/blender/editors/interface/interface_draw.cc b/source/blender/editors/interface/interface_draw.cc index 8a377c83772..ca47cfcd235 100644 --- a/source/blender/editors/interface/interface_draw.cc +++ b/source/blender/editors/interface/interface_draw.cc @@ -1005,78 +1005,38 @@ void ui_draw_but_VECTORSCOPE(ARegion * /*region*/, immUnbindProgram(); /* Restore scissor test. */ GPU_scissor(UNPACK4(scissor)); /* outline */ draw_scope_end(&rect); GPU_blend(GPU_BLEND_NONE); } -static void ui_draw_colorband_handle_tri_hlight( +static void ui_draw_colorband_handle_tri( uint pos, float x1, float y1, float halfwidth, float height) { - GPU_line_smooth(true); - - immBegin(GPU_PRIM_LINE_STRIP, 3); + immBegin(GPU_PRIM_TRIS, 3); immVertex2f(pos, x1 + halfwidth, y1); immVertex2f(pos, x1, y1 + height); immVertex2f(pos, x1 - halfwidth, y1); immEnd(); - - GPU_line_smooth(false); } -static void ui_draw_colorband_handle_tri( - uint pos, float x1, float y1, float halfwidth, float height, bool fill) +static void ui_draw_colorband_handle_box(uint pos, float x1, float y1, float x2, float y2) { - if (fill) { - GPU_polygon_smooth(true); - } - else { - GPU_line_smooth(true); - } - - immBegin(fill ? GPU_PRIM_TRIS : GPU_PRIM_LINE_LOOP, 3); - immVertex2f(pos, x1 + halfwidth, y1); - immVertex2f(pos, x1, y1 + height); - immVertex2f(pos, x1 - halfwidth, y1); + immBegin(GPU_PRIM_TRI_STRIP, 4); + immVertex2f(pos, x2, y1); + immVertex2f(pos, x1, y1); + immVertex2f(pos, x2, y2); + immVertex2f(pos, x1, y2); immEnd(); - - if (fill) { - GPU_polygon_smooth(false); - } - else { - GPU_line_smooth(false); - } -} - -static void ui_draw_colorband_handle_box( - uint pos, float x1, float y1, float x2, float y2, bool fill) -{ - if (fill) { - immBegin(GPU_PRIM_TRI_STRIP, 4); - immVertex2f(pos, x2, y1); - immVertex2f(pos, x1, y1); - immVertex2f(pos, x2, y2); - immVertex2f(pos, x1, y2); - immEnd(); - } - else { - immBegin(GPU_PRIM_LINE_STRIP, 5); - immVertex2f(pos, x1, y1); - immVertex2f(pos, x1, y2); - immVertex2f(pos, x2, y2); - immVertex2f(pos, x2, y1); - immVertex2f(pos, x1, y1); - immEnd(); - } } static void ui_draw_colorband_handle(uint shdr_pos, const rcti *rect, float x, const float rgb[3], ColorManagedDisplay *display, bool active) { const float sizey = BLI_rcti_size_y(rect); @@ -1120,62 +1080,68 @@ static void ui_draw_colorband_handle(uint shdr_pos, /* hide handles when zoomed out too far */ if (half_width < min_width) { return; } } /* shift handle down */ y1 -= half_width; + /* Black outline around the lower box. */ immUniformColor3ub(0, 0, 0); - ui_draw_colorband_handle_box( - shdr_pos, x - half_width, y1 - 1, x + half_width, y1 + height, false); + ui_draw_colorband_handle_box(shdr_pos, + x - half_width - U.pixelsize, + y1 - U.pixelsize, + x + half_width + U.pixelsize, + y1 + height); + + /* Grey box inset by line width. */ + immUniformColor3ub(128, 128, 128); + ui_draw_colorband_handle_box(shdr_pos, x - half_width, y1, x + half_width, y1 + height); - /* draw all triangles blended */ + if (display) { + IMB_colormanagement_scene_linear_to_display_v3(colf, display); + } + + /* Color value inset by another line width. */ + immUniformColor3fv(colf); + ui_draw_colorband_handle_box(shdr_pos, + x - (half_width - U.pixelsize), + y1 + U.pixelsize, + x + (half_width - U.pixelsize), + y1 + height - U.pixelsize); + + /* draw all triangles blended and smooth. */ GPU_blend(GPU_BLEND_ALPHA); + GPU_polygon_smooth(true); - ui_draw_colorband_handle_tri(shdr_pos, x, y1 + height, half_width, half_width, true); + /* Black outline around the top triangle. */ + immUniformColor3ub(0, 0, 0); + ui_draw_colorband_handle_tri( + shdr_pos, x, y1 + height, half_width + U.pixelsize, half_width + U.pixelsize); + /* Triangle main color. */ if (active) { immUniformColor3ub(196, 196, 196); } else { immUniformColor3ub(96, 96, 96); } - ui_draw_colorband_handle_tri(shdr_pos, x, y1 + height, half_width, half_width, true); - - if (active) { - immUniformColor3ub(255, 255, 255); - } - else { - immUniformColor3ub(128, 128, 128); - } - ui_draw_colorband_handle_tri_hlight( - shdr_pos, x, y1 + height - 1, (half_width - 1), (half_width - 1)); - - immUniformColor3ub(0, 0, 0); - ui_draw_colorband_handle_tri_hlight(shdr_pos, x, y1 + height, half_width, half_width); + ui_draw_colorband_handle_tri(shdr_pos, + x, + y1 + height, + half_width - (0.5f * U.pixelsize), + half_width - (0.5f * U.pixelsize)); + GPU_polygon_smooth(false); GPU_blend(GPU_BLEND_NONE); - - immUniformColor3ub(128, 128, 128); - ui_draw_colorband_handle_box( - shdr_pos, x - (half_width - 1), y1, x + (half_width - 1), y1 + height, true); - - if (display) { - IMB_colormanagement_scene_linear_to_display_v3(colf, display); - } - - immUniformColor3fv(colf); - ui_draw_colorband_handle_box( - shdr_pos, x - (half_width - 2), y1 + 1, x + (half_width - 2), y1 + height - 2, true); } void ui_draw_but_COLORBAND(uiBut *but, const uiWidgetColors * /*wcol*/, const rcti *rect) { ColorManagedDisplay *display = ui_block_cm_display_get(but->block); uint pos_id, col_id; uiButColorBand *but_coba = (uiButColorBand *)but; ColorBand *coba = (but_coba->edit_coba == nullptr) ? (ColorBand *)but->poin : but_coba->edit_coba; ```

image.png

13 KiB

Philipp Oeser commented

2023-08-31 10:02:40 +02:00

Tada! Exactly what the patch is doing, no?

For sure. But the same issue happens with the triangles, which also have to be drawn filled. And I am trying to get "Line Width" working as expected. With the following I can get it like the following (Thin, Default, Thick):

Ah, the triangle part of the widget (forgot about that one).
Yeah, that should be done, too, agree with your changes.

Since we already took the deep dive (wasnt expecting this to swallow that much time -- but in the end this is good!), the one remaining thing that still confuses me is this part from my PR description:

Both ui_draw_colorband_handle_box and imm_draw_box_wire_2d with GPU_PRIM_LINE_STRIP seem to only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weird and I could not track it down

So for maximum educational purposes, we could still investigate that (not necessary though, just a bonus), otherwise, I could update this PR with the relevant code, or you could take over (whatever you prefer).

> > Tada! Exactly what the patch is doing, no? > > For sure. But the same issue happens with the triangles, which also have to be drawn filled. And I am trying to get "Line Width" working as expected. With the following I can get it like the following (Thin, Default, Thick): Ah, the triangle part of the widget (forgot about that one). Yeah, that should be done, too, agree with your changes. Since we already took the deep dive (wasnt expecting this to swallow that much time -- but in the end this is good!), the one remaining thing that still confuses me is this part from my PR description: >Both `ui_draw_colorband_handle_box` and `imm_draw_box_wire_2d` with `GPU_PRIM_LINE_STRIP` seem to only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weird and I could not track it down So for maximum educational purposes, we could still investigate that (not necessary though, just a bonus), otherwise, I could update this PR with the relevant code, or you could take over (whatever you prefer).

Harley Acheson commented

2023-08-31 17:30:32 +02:00

Both ui_draw_colorband_handle_box and imm_draw_box_wire_2d with GPU_PRIM_LINE_STRIP seem to only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weird and I could not track it down

Do you have an example of GPU_PRIM_LINE_STRIP working differently elsewhere? It was only recently that I noticed the line drawing (when greater than a single pixel) leaves gaps at the corners.

Replacing some of our base drawing routines for unfilled boxes would make some sense, using filled rectangles for the lines when over a pixel in width.

otherwise, I could update this PR with the relevant code, or you could take over (whatever you prefer).

It mostly depends on how much you are enjoying this. If you have had enough of staring at these things you could update and we could be done.

But if you are having fun the next logical steps are to replace the filled boxes with UI_draw_roundbox_4fv. Using that could round off the corners a bit. And then as these are drawn smaller reduce some of the sizes/complexity when it gets too small. Right now it just gets super clunky and then disappears when scaled down.

If the above sounds like a more trouble than it is worth, just ignore it, update the PR with that we have and we can be done.

Or if you want the above AND you are also bored of this then I can take it and play with it on the weekend.

> >Both `ui_draw_colorband_handle_box` and `imm_draw_box_wire_2d` with `GPU_PRIM_LINE_STRIP` seem to only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weird and I could not track it down Do you have an example of GPU_PRIM_LINE_STRIP working differently elsewhere? It was only recently that I noticed the line drawing (when greater than a single pixel) leaves gaps at the corners. Replacing some of our base drawing routines for unfilled boxes would make some sense, using filled rectangles for the lines when over a pixel in width. > otherwise, I could update this PR with the relevant code, or you could take over (whatever you prefer). It mostly depends on how much you are enjoying this. If you have had enough of staring at these things you could update and we could be done. But if you are having fun the next logical steps are to replace the filled boxes with UI_draw_roundbox_4fv. Using that could round off the corners a bit. And then as these are drawn smaller reduce some of the sizes/complexity when it gets too small. Right now it just gets super clunky and then disappears when scaled down. If the above sounds like a more trouble than it is worth, just ignore it, update the PR with that we have and we can be done. Or if you want the above AND you are also bored of this then I can take it and play with it on the weekend.

Philipp Oeser commented

2023-09-05 11:27:01 +02:00

Both ui_draw_colorband_handle_box and imm_draw_box_wire_2d with GPU_PRIM_LINE_STRIP seem to only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weird and I could not track it down

Do you have an example of GPU_PRIM_LINE_STRIP working differently elsewhere? It was only recently that I noticed the line drawing (when greater than a single pixel) leaves gaps at the corners.

I just wasnt noticing any such issues with imm_draw_box_wire_2d (and that uses a very similar method, just with GPU_PRIM_LINES).
I am pretty sure I even tested imm_draw_box_wire_2d for the colorramp then (and there it also misbehaved...)

In any case, this is now superseeded by !111903 , so will close.
Thx taking over @Harley !

> > >Both `ui_draw_colorband_handle_box` and `imm_draw_box_wire_2d` with `GPU_PRIM_LINE_STRIP` seem to only act wrong in the context of the colorband though (and are working fine elsewhere) which is a bit weird and I could not track it down > > Do you have an example of GPU_PRIM_LINE_STRIP working differently elsewhere? It was only recently that I noticed the line drawing (when greater than a single pixel) leaves gaps at the corners. I just wasnt noticing any such issues with `imm_draw_box_wire_2d` (and that uses a very similar method, just with GPU_PRIM_LINES). I am pretty sure I even tested `imm_draw_box_wire_2d` for the colorramp then (and there it also misbehaved...) In any case, this is now superseeded by !111903 , so will close. Thx taking over @Harley !

Philipp Oeser closed this pull request

2023-09-05 11:27:03 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #111017: Gap between color square and frame of box in the color ramp #111168

Pull request closed