Combine ScaledGraphics when layering them

[akoch-yatta]

This PR improves layering of ScaledGraphics but not creating several ScaledGraphics on top of each other but aggregating each new ScaledGraphics instance to inherit the state of the parent. This is probably a way simpler solution than #834 and could make the changes to ScaledGraphics there unnecessary. Due to my testing all looks fine with this approach and it would improve the handling of font scaling (that was the one limitation from #834).

Do you see any possible problems with this?

[ptziegler]

I'm not sure if this can really work. The first problem is that ScaledGraphics can be subclasses, so that would have to be included in the check.

But more generally speaking, I'm not sure whether it really is such a great idea to mix the state of multiple ScalableGraphics instances. The "parent" ScaledGraphics instance doesn't keep a reference to the "child" ScaledGraphics. So it's effectively impossible to reason about what contributes to the combined zoom level.

[akoch-yatta]

I'm not sure if this can really work. The first problem is that ScaledGraphics can be subclasses, so that would have to be included in the check.

Fair point, that could be covered with a stricter check. Initially I wanted to put this logic not into the constructor, but into the IScalablePaneHelper#prepareScaledGraphics, but I cannot access all fields there.

But more generally speaking, I'm not sure whether it really is such a great idea to mix the state of multiple ScalableGraphics instances. The "parent" ScaledGraphics instance doesn't keep a reference to the "child" ScaledGraphics. So it's effectively impossible to reason about what contributes to the combined zoom level.

But in which scenario is this necessary? The implementation logic in the constructor is tightly coupled to what scaling affects in ScalableGraphics, so we could create issues with a subclass as you mentioned above. To be fair, most issues are already solved with #834.

[ptziegler]

But in which scenario is this necessary?

When something breaks, I suppose. Because that's where you'll have a hard time figuring out what exactly contributed to the total zoom of the ScaledGraphics instance. I'm willing to give it a try for the M1, I just want to point out that this might very easily lead to obscure edge-cases that are very hard to debug.

And as you said, this change effectively makes #834 obsolete, because you no longer have a ScaledGraphics instance embedded in another ScaledGraphics instance. So you might as well revert this change alltogether.