Page MenuHome

UI: Don't draw icons if the widget is too small
Needs RevisionPublic

Authored by Hans Goudey (HooglyBoogly) on Sat, Mar 14, 4:38 PM.

Details

Summary

It's a rare situation but it looks messy to have a bunch of icons not fit in their buttons when they are too small.

I've mostly noticed this in the modifier properties panel, but it happens a bit elsewhere too.

Before / After

Diff Detail

Event Timeline

Hey, that's a cool idea!

To my eye though, I think your test isn't quite right. It is not showing the icon when the widget gets narrower than the width of the icon. But the icon is generally shown with a small amount of padding (can't remember the amount off the top of my head). So there would still be some narrow widths that would be wider than that test that would have some icon overlap like this:

But still seems like a good idea if you just increase that by that padding amount.

Just for giggles though... have you tried altering the size of the icon? The "aspect" immediately after your code change can be altered to make the icon smaller once the you go under that width threshhold. Note that you'd also have to alter the vertical placement then too. Not that it is a better solution, but might be fun to explore...

Thanks for looking at this!

IIRC correctly the icons are actually 14x14 pixels, so there is 2px of border. Indeed subtracting 2 * UI_DPI_FAC from the left side of the comparison seems to look a bit better. I don't like that "2" magic number though, got to find somewhere the padding is defined in the code.

I think scaling the icons down looks a bit odd.

This isn't the only problem to solve when things get shrunk down like this, but it does feel like a pretty straightforward change to help.

I'm not sure that's a good change.

Now, it doesn't look good, but at least you can understand what these buttons do. With this change, you have a row of exactly the same buttons, which is useless.

Also, buttons without EMBOSS become completely invisible.

Maybe it would be better to:

  1. Don't change the behavior for non-embossed buttons.
  2. Instead of not drawing the icon at all, crop the parts that appear outside of the boundary.

How does that sound?

Maybe it would be better to:

  1. Don't change the behavior for non-embossed buttons.
  2. Instead of not drawing the icon at all, crop the parts that appear outside of the boundary.

How does that sound?

+1

For the cropping you could use GPU_scissor(), although better avoid calling if it's not needed (do bound checks first).

For the cropping you could use GPU_scissor(), although better avoid calling if it's not needed (do bound checks first).

Definitely do not call that, it's bad for performance.

Brecht Van Lommel (brecht) requested changes to this revision.Thu, Mar 26, 2:28 PM

Marking as request changes so it's out of the to-be-reviewed queue.

Really the proper solution here is to enforce minimum width for regions so this doesn't happen at all.

I think scaling down is probably the most efficient solution, adding some kind of cropping logic is likely to affect performance also for the non-cropping cases.

This revision now requires changes to proceed.Thu, Mar 26, 2:28 PM

For the cropping you could use GPU_scissor(), although better avoid calling if it's not needed (do bound checks first).

Definitely do not call that, it's bad for performance.

Are you sure about this? Searching online I didn't see any objections against using glScissor() for performance reasons. In fact results suggested that it shouldn't have noticeable performance impacts at all.

ImmGUI calls it for each draw command as part of its main OpenGL geometry draw function (https://github.com/ocornut/imgui/blob/master/examples/imgui_impl_opengl3.cpp#L364-L386).
Checking the Qt code, they use it all the time to define clipping bounds, from what I see they always call it when drawing text, icons or most other geometry. Of course I could be wrong. It's also recommended as "by far the fastest way to do clipping" here https://www.qt.io/blog/2010/01/06/qt-graphics-and-performance-opengl. The author seems like a trustworthy source.
We could of course benchmark this.

Marking as request changes so it's out of the to-be-reviewed queue.
Really the proper solution here is to enforce minimum width for regions so this doesn't happen at all.

How would that work? A reasonable minimum width depends a lot on the layout. I have a hard time imagining how we could enforce a minimum size on area size changes, especially if caused through window resizing.

We want to be able to batch draw calls. This is already happening for text, not for icons currently, but using scissor would prevent doing that in the future.