ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
59.37k stars 10.11k forks source link

ScaleAllSizes and DPI scaling issues #5452

Open petrihakkinen opened 2 years ago

petrihakkinen commented 2 years ago

Hi! I've recently implemented DPI scaling to my Shinobi engine and I think DPI support in ImGui could be improved. Most notably ScaleAllSizes is problematic for a couple of reasons:

1) It's destructive. Let's say we have a slider for changing the DPI scale. The app has to maintain two sets of styles: the current style with DPI scaled sizes and original non-scaled style. When the DPI changes the original style needs to be copied over the current style before calling ScaleAllSizes (or the style has to be somehow reset by other means). This is quite cumbersome and when styles are modified at runtime the changes usually need to be applied to both copies (to "current" to see the effect and to "original" so that the next DPI scale change don't overwrite the modifications).

(The other simpler alternative is to scale the variables by newscale/oldscale but if this is done repeatedly, for example when the slider is moved, the numeric precision of style variables will degrade over time.)

2) For any PushStyleVar calls the client code has to multiply the values with current DPI scale factor. This is again cumbersome and easy to forget and can easily cause style inconsistencies that may go undetected for a while (and may be hard to fix in old code).

I propose adding a new global style variable "DPI scale factor" which is automatically applied to every style variable when the variable is read by ImGui. This would move the complexity of maintaining the styles from every app to ImGui itself.

ocornut commented 2 years ago

I propose adding a new global style variable "DPI scale factor" which is automatically applied to every style variable when the variable is read by ImGui. This would move the complexity of maintaining the styles from every app to ImGui itself.

We've been exploring different ways of handling DPI.

I believe this is a probable good solution (and generally sane), but we haven't nailed where to put the responsibility of rounding values. Many rendering primitives assume pixel alignments. Putting that responsibility on widgets makes it difficult to iterate on a solution later, and naive rounding or flooring may lead to items with unstable size (between e.g. W and W+1) and preventing situation where smooth zooming is desirable (sometimes we don't want to align the primitives). Putting that responsibility at lower-level (e.g. ImDrawList) is not always desirable either. Add to the mix that not all code is currently functioning correctly with non-integer positions. I haven't had time to explore this thoroughly yet.

ocornut commented 2 years ago

Addenum: Add to the mix the fact that when we go full on "any scaling values" we are likely to end up with MOST positions not aligning on integer which is although fine/correct from a technical perspective will basically make practically everything a little more confusing to deal with (using a debugger, printing values,, storing ini values).

If layout function aligns output to a a grid (e.g. pixel) that would alleviate this but then we loose the ability to have a 100% jiggle-free smooth smoothing. My intuition is that we might need two modes, one for animating zooming and one for stable rendering.

SuperWangKai commented 2 years ago

What about using float to record the values used for scaling calculation and the rounded values for drawing and stuff for ImGui?

petrihakkinen commented 2 years ago

I believe this is a probable good solution (and generally sane), but we haven't nailed where to put the responsibility of rounding values. Many rendering primitives assume pixel alignments. Putting that responsibility on widgets makes it difficult to iterate on a solution later

You could add getters for DPI scaled values to the style struct. That way the details of flooring/rounding/no rounding are in one place and different options can be easily explored if needed.

(Added bonus: not sure how you're handling (going to handle?) multiple DPIs but with the getters you could pass the current DPI scale factor to the getters as an argument in the future, if needed.)