BR: fillAll doesn't fill all

Didn’t @jules just explain that it’s not a bug and that fillAll() is filling the entire context?

The issue is that if the edges of the component do not align with pixel bondaries then when the graphics context is clipped, any pixels drawn at that boundary are anti-aliased and so are drawn semi-transparent. It’s less about “fillAll() doesn’t do what it says” but more about the nuances of the clipping behavior. If you were to draw an image, or any other sort of graphics to the edge of the component I’d guess you’d see the same thing of the results of previous calls “leaking” through like this.


To me it seems like the thing that’s not working here is that each drawing call to the graphics context is individually being anti-aliased. So when you draw the red background, that’s being clipped to the component’s bounds which is drawing a semi-transparent column of pixels on that far-right edge. Later, you’re drawing the grey pixels on-top and those are separately being anti-aliased and so again the far-right edge is semi-transparent, resulting in the red showing through.

Instead, it would be ideal for the anti-aliasing to only be done once after the paint() call has finished - so the first call that draws th red would draw solid pixels that spill out over the component’s bounds, as would the second call to fill the grey. Then the anti-aliasing would be applied to that final set of pixels which would result in the expected behaviour of the semi-transparent grey on the edge that’s not perfectly aligned.

2 Likes

Interestingly, this is reproducable in Affinity Designer (poor man’s Illustrator):

Two rectangles each with a border of 1.5px. Red below, black on top:

When rendered as a vector, there’s no visible issues (unless you zoom in really-really close on the rounded corners, you can see a few red specs peaking through!), however when you enable rasterisation:

You see a similar issue of the red background “leaking” out from behind the black, even though it shouldn’t be visible.

Photoshop however seems to get it right:

As does Figma:

So if this is to be considered a bug, it’s more a bug with the way JUCE renders multiple layers with a non-integer clip region. It’s not a bug with fillAll().

2 Likes

Can we all agree that if you intentionally fill a rect that is bigger than the clip rect, that is gets clipped correctly and thus the fillRect call fills every pixel (blended or not) for the entire clip rect?

Can we also agree that fillAll does behave differently?

If we know both things to be true, then the only correct conclusion is that fillAll is NOT doing what it says and thus is buggy?

Can we also be realistic here? Why talk about ideal algorithms, conversions to float, etc. if there is a one-line workaround that archives the desired goal and would fix the bug?

Would it be too much to ask to fix the fillAll function to get the clip rect, call “expanded(1)” on it and call it a day?

1 Like

I tend to use Rectangle:: toNearestIntEdges() when setting the Component bounds on a resize… perhaps that may be a workaround until this can be fixed?

I tend to use Component::setPaintingIsUnclipped() set to true to improve performance… so painting has to be limited to inside the bounds of the Component.

Rail

2 Likes

Without the expansion, you get the same issue with the red showing through, just not as strongly as the red is also semi-transparent. It’s not that the clipping is being done “correctly” with the red - the same clip region is present for both the fillRect call and fillAll. The difference is that in fillAll(), a call to context.fillRect is made using the bounds of the clip region, which is 0, 0, 101, 100. However, becuase scaling is involved, the actual clip region is likely not aligned to pixel boundaries and so the region that’s returned by context.getClipRegion() has been rounded (which is apparent by the fact that a component of size 100x100 has a clip region with a size of 101x100 - clearly something’s gone wrong there).

Yes, but the issue lies further down, not in the fillAll() function itself.

It’s filling the whole clip region as described by LowLevelGraphicsContext::getClipRegion(), which is what the docs say and what its name implies it does.

Because that’s a workaround, and workarounds != solutions.

It’s not filling the entire context. Take this example: https://github.com/FigBug/juce_bugs/blob/master/DrawingGlitch2/Source/MainComponent.h

I have two components the exact same size and the exact same colour. I fill one with fillAll() and the other with fillRect() the one with fillRect() draws larger. So if fillAll() is drawing correctly, then the rest of the drawing commands are broken and drawing outside of the the clip rect.

You can see with zoomed in image, fillAll() is anti aliased on the edge where fillRect() is not.

1 Like

Again, I think the issue is that since the clip region being reported by context.getClipBounds() is using ints, but there’s a non-integer scaling going on, the “true” clip region being used by the underlying renderer is likely using floats, and its bounds don’t align with the clip region that JUCE is reporting. The biggest clue being that the clip region has a height of 101 and a width of 100, despite the component being 100x100 - so clearly there’s something fishy going on.

Your component originally had the bounds {209, 194, 100, 100}. After scaling, that becomes {146.3, 135.8, 70, 70} which is what I think the underlying renderer must be using. When we then call getClipRegion() we get {0, 0, 100, 101} which I can only assume means something’s gone wrong when reversing the scaling. So, when you then call fillAll(), you’re asking the underlying context to fill the area {0, 0, 100, 101}, and, because that area has been rounded to a pixel boundary, what the underlying renderer ends up drawing is a recangle that’s slightly to the left and slightly higher up than the “true” clip region - but because the height has somehow ended up at 101 you only see the artifact on the right edge.

So, I don’t necessarily disagree that fillAll() isn’t filling the entire clip region - it’s just that it is filling the entire region that it’s told is the clip region, but that region is not the “true” clip region being used by the underlying renderer.

@RolandMR

Bit surprising that you’d see a difference between fillAll and fillRect when you look at how it’s implemented!

void Graphics::fillAll() const
{
    fillRect (context.getClipBounds());
}

Although I stick to my point above that this is fundamentally just always going to be a problem if your bounds aren’t pixel-aligned, I wonder if what you’re seeing there could be anti-aliasing being applied twice? i.e. anti-aliasing the rectangle and then anti-aliasing it again when it clips it…? TBH I doubt that because what I think it does is just clip the coordinates and draw it once, but I guess it’s not impossible.

There’s a difference between the two fillAll() methods - the one being invoked here is:

void Graphics::fillAll (Colour colourToUse) const
{
    if (! colourToUse.isTransparent())
    {
        auto clip = context.getClipBounds();

        context.saveState();
        context.setFill (colourToUse);
        context.fillRect (clip, false);
        context.restoreState();
    }
}

Sure, but that just changes the colour, it shouldn’t make any difference to the clipping or the rectangle being drawn (…unless of course there’s some kind of obscure side-effect lurking in the state save/load functions)

That is exactly what is happening. The clip rect is already on float coordinates. But the fillRect inside fillAll is using these so it draws with anti aliasing and the non int edges get anti aliased before the clipping anti aliases them again.

All that needs to be changed inside the fillRect calls is to extend the clipRect to the nearest int edges (floor for top/left, ceil for bottom/right) and the problem would be solved. Only the clipping itself is doing any anti aliasing. The rect itself would be solid.

That’s still only a workaround, it’s not a solution…

The bug here is that context.getClipBounds() isn’t reporting the correct bounds and so the area you’re requesting to fill isn’t the correct area - how can {0, 0, 100, 101} be the correct clip bounds for a component with a size of 100x100!?

The clip bounds are returned the same way components use int coordinates. It’s all int coordinates but they get transformed at the appropriate draw calls, so ultimately, all draw calls result in float coordinates.

Components = int
clipRect = int

Actual drawing = transformed into float coordinates

So the clip rect says, I’m 100x100. The fillAll then also uses a 100x100 rect. Now both, the clip rect and the fillAll rect gets transformed and thus end up being 133.33f. Now the fill only targets 133.33f and the clipping also happens at 133.33f. Now the anti-aliasing is happening twice. 0.33f * 0.33f = 0.1089. The alpha value of the non-integer edges result in too weak an alpha value.

No other functions are affected because you have to expect that transforms lead to this kind of problem. In our projects, we use ONLY float coordinates for all drawing functions, so we have total control and can tweak our look accordingly.

Only the fillAll uses int coordinates internally, thus leading to this particular problem. Since it’s the only affected call, it’s completely prudent to correct the rectangle, to avoid this problem.

Your “solution” would mean that the graphics context needs to return a transformed rectangle in float coordinates, then you would need to turn of the anti-aliasing for the clip itself so that the fill can do its job correctly. Then after, the clipping needs to be turned back on again.

Extending the rectangle used for the fillRect is much easier, faster, and doesn’t affect ANYTHING else. It’s a much safer solution to this problem.

That’s just not what’s happening - if you step into fillAll() you’ll see the result of context.getClipBounds() returns {0, 0, 100, 101} which is different from the component’s bounds!

That wouldn’t lead to the results we’re seeing. If it were filling one more pixel at the bottom, you wouldn’t see that “underfill” problem.

So now we’re talking about a potential second bug. Unrelated to the underfill problem.

But that’s exactly what we are seeing… the red is showing through on the right-edge because the clip region is 100 wide, but not showing on the bottom edge because the clip region is 101 tall…

That’s the only bug I see here! If you fix that so that the clip region is 100x100 then you’d see the red showing through on the right and the bottom which is what you’d expect to see given the way JUCE renders each draw call in a separate layer. The suggestion you’ve proposed would technically be drawing outside the correct region since you’re effectively rounding up to fill that last pixel - which isn’t fully within the clip region after scaling’s applied.

But it’s still clipped by the low-level context regardless of what rectangle you feed into the fillRect call. You can safely feed {-10000,-10000, 1000000, 1000000 } in there, and it will be correctly clipped. Or do you think that any fillRect automatically just does that, regardless of the clip rect and then overstomps memory like crazy?

What we should be seeing is:

clip_bounds_bug

What we’d ideally see, by fixing the red showing through, is:

clip_bounds_bug_no_red

However, what we are seeing, because of the integer clipping, is:

clip_bounds_bug_actual

Then you don’t understand the problem at all.

With a correct “fillAll”, we shouldn’t see any color fringes anywhere.