Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dirty propagation (i.e. IRECT::Intersects()) doesn't commute and is inconsistent with draw/rendering #1009

Open
svantana opened this issue Oct 7, 2023 · 7 comments

Comments

@svantana
Copy link
Contributor

svantana commented Oct 7, 2023

The following code:

IRECT A(0,0,10,10), B(10, 5, 20.6f, 15), C(20.7f, 10, 30, 20);
g.FillRect(COLOR_RED  , A);
g.FillRect(COLOR_GREEN, B);
g.FillRect(COLOR_BLUE , C);
printf("(A,B)=%d, (B,A)=%d, (B,C)=%d, (C,B)=%d\n", A.Intersects(B), B.Intersects(A), B.Intersects(C), C.Intersects(B));

gives: (A,B)=1, (B,A)=0, (B,C)=0, (C,B)=0

and looks like this
Screenshot 2023-10-07 at 11 55 43

To be consistent, Intersects() should expand the bounds to integer screen pixels and use strict inequality instead.

Does this matter? Not much, but an annoyance is that neighboring controls without spacing trigger redraw on eachother unnecessarily.

@AlexHarker
Copy link
Collaborator

AlexHarker commented Oct 7, 2023

Thanks for raising this - there are some things to discuss to disentangle the two issues that are combined here:

1 - Can you clarify what you mean by "Dirty Propagation"?

I fear here that you might in part have some (incorrect) assumptions about how controls are marked dirty in iplug 2 that I hope will become clearer below.

2 - "To be consistent, Intersects() should expand the bounds to integer screen pixels"

I disagree here - consistent with what exactly?

When iPlug2 is drawing it calculates dirty intersections by snapping to pixel alignment first (and prior to that padding control rects slightly -

IRECT controlBounds = pControl->GetRECT().GetPadded(0.75).GetPixelAligned(scale);
You'll notice that pixel alignment takes into account drawing size - if there's a situation where controls are not correctly being marked dirty please give an example, as I think that is unlikely and we (I) spent quite a long time on that for iPlug2.

The Intersects() method should tell you whether two rects intersect or not in abstract space (in fact the docs often say pixels due to the history of iPlug but mostly it's in arbitrary units that may or may not be screen pixels - it knows nothing about how the IRECTs will be displayed - and nor should it). If you want to know about display things that gets more complex but you are free to do your pixel alignment first (using GetPixelAligned() or any of the related methods (but not PixelSnap()) which would give you the behaviour you want in terms of pixels.

3 - "and use strict inequality instead"

I am guessing this relates to the lack of commutivity. This definitely seems like an issue to me - if you'd like to propose an alteration to Intersects()

inline bool Intersects(const IRECT& rhs) const
then that's definitely something we could consider - especially if you can demo the change of results.

@AlexHarker
Copy link
Collaborator

In summary (and it would be good to get @olilarkin's thoughts here), it seems that when two IRECTs are touching they only return as intersecting when the test is run one way (so the results are non-commutative). I would suggest we rethink what the definition of intersecting should be with this in mind. The correct results in the OP should be EITHER:

(A,B)=1, (B,A)=1, (B,C)=0, (C,B)=0 [touching IRECTs intersect but non-touching ones don't]

OR

(A,B)=0, (B,A)=0, (B,C)=0, (C,B)=0 [touching IRECTs don't intersect and neither do non-touching ones]

but what we have at the moment does seem non-ideal.

In cases where pixel alignment matters then the user needs to take that into account themselves (iplug2 already does so where it needs to).

@svantana
Copy link
Contributor Author

svantana commented Oct 7, 2023

Thanks for the great input @AlexHarker ! I definitely didn't look hard enough at the code.

The GetPadded(0.75) seems kind of arbitrary to me. Is that some kind of worst case estimate? If so, that should at least by in pixel space. To me, it would make more sense that the mRECT bounds were hard, so that even antialiasing would be clipped at the user-set bounds (expanded to pixel integers). iPlug2 is hardly for beginners, so we should trust it's users to know what they are doing.

Sidenote: this explains another annoyance I've had - that IControls with zero width are still visible.

@AlexHarker
Copy link
Collaborator

AlexHarker commented Oct 7, 2023

The 0.75 is to account for drawing an outline of 1 "user pixel" (i.e. before draw scaling) on the bounds of the IRECT which is done in live edit mode, and which some users may wish to do on a control (if you want a bigger boundary you need to set your control bounds differently). In theory that should be 0.5 (because the line is centred on the bounds), so I'm not sure what the extra 0.25 is for - that does seem a little arbitrary, but I suspect I saw issues using 0.5 exactly, possibly due to floating point accuracy issues.

If you click through to the line linked above you'll see a comment about the 0.75...

@svantana
Copy link
Contributor Author

svantana commented Oct 7, 2023

Ah, I see, thanks for the clarification. To me, the notion of wanting to draw outside of the bounds is foreign, but that could at least be an optional setting in IGraphics (SetControlPadding or such). My main issue is wanting pixel-perfectly bordering IControls that do not trigger eachother's redraw.

A related nice-to-have is an "opaqueness" property of IControls, so that background draws can be skipped (and the dirty loop need not even run).

@AlexHarker
Copy link
Collaborator

There is no way to guarantee what you want unless your plugin display is strictly limited to display at 100% scale (or multiples) as controls will not be pixel aligned with respect to the display in many other scaling conditions. Do you have a situation that displays a distinct issue in terms of redrawing in terms of performance?

@svantana
Copy link
Contributor Author

svantana commented Oct 7, 2023

I try to account for the scaling ratio when setting up the rects so that borders will be sharp -- a bit tricky to account for float rounding but it's possible.

The performance slowdown may not be a big deal, but on the other hand, why not aim for maximum performance? My main annoyance is debugging and such - a control next to a level meter will draw constantly, so it requires some extra steps to catch the intentional redraw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants