Skip to content

back/cairo: keep the exact clip across a graphics state save#158

Open
DTW-Thalion wants to merge 2 commits into
gnustep:masterfrom
DTW-Thalion:fix/cairo-clip-save-restore
Open

back/cairo: keep the exact clip across a graphics state save#158
DTW-Thalion wants to merge 2 commits into
gnustep:masterfrom
DTW-Thalion:fix/cairo-clip-save-restore

Conversation

@DTW-Thalion

Copy link
Copy Markdown
Contributor

Fixes #68.

-[CairoGState copyWithZone:] rebuilt the clip from cairo_copy_clip_rectangle_list, which reports a non-rectangular clip as CLIP_NOT_REPRESENTABLE and fell back to the clip bounding box. A clip set before -saveGraphicsState was widened to its bounding box on the copied state, so an NSGradient drawn in a bezier path spilled outside the shape (libs-gui #228).

Track the clip paths in base space as they are applied in -DPSclip and -DPSeoclip, drop them in -DPSinitclip and -DPSinitgraphics, and replay them onto the copied context. This keeps the separate per-state cairo_t that GSDefineGState and DPSsetgstate rely on, so it avoids the scrolling and image regressions the earlier cairo_save/cairo_restore approach caused.

Tests/cairo/clipsaverestore.m covers triangle, oval and even-odd ring clips across save and restore; it fails on master (the non-rectangular clips widen to their bounding box) and passes with the fix. The rest of the Tests/cairo suite is unchanged.

-[CairoGState copyWithZone:] rebuilt the clip from
cairo_copy_clip_rectangle_list, which reports a non-rectangular clip as
CLIP_NOT_REPRESENTABLE and then fell back to the clip bounding box.  A clip
set before -saveGraphicsState was therefore widened to its bounding box on
the copied state, so drawing an NSGradient in a bezier path spilled outside
the intended shape.

Track the clip paths in base space as they are applied in -DPSclip and
-DPSeoclip, drop them in -DPSinitclip and -DPSinitgraphics, and replay them
onto the copied context so the exact clip is reproduced.  This keeps the
separate per-state cairo_t that the opaque gstate operations (GSDefineGState,
DPSsetgstate) depend on, which is why cairo_save/cairo_restore cannot be used
for this instead.

Add Tests/cairo/clipsaverestore.m covering triangle, oval and even-odd ring
clips across save and restore.
@fredkiefer

Copy link
Copy Markdown
Member

This is a very impressive change. I will have to run it locally to see, if there are any downsides.
I think that your test code at the moment won't detect any real changes, just the missing log message. What would be needed is some more drawing after the restore to confirm that the actual clip is still in place and it didn't get extended to a rectangle.
And if it really works we should try to do something similar on the GSGstate (with the NSBezierPath directly) to have that issue fixed for all backends. But I am still a bit reluctant.

@DTW-Thalion

Copy link
Copy Markdown
Contributor Author

Test away ! I am not exhaustively testing these in live environments, so while they seem to work based on whatever unit tests I can muster and I try functional testing where I can, the devil is in the details, and yes we can look at GSGState if you think this will hold up to scrutiny.

@DTW-Thalion

DTW-Thalion commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Just an update: I ran it against both backends to double-check. On the current master backend three of the six fail — the triangle and oval bleed past the real clip into its bounding box after the save, and the even-odd ring drops out; on the branch all six pass. So the fill after the save does land on the copied gstate and exercises the real clip shape, not just the log path.

One gotcha that cost me a while: the loaded backend is the .cairo bundle variant, so a stale installed bundle makes old and new render identically and the test looks inert — a clean rebuild/install sorts it.

A draw-after-restore check would land on the restored original gstate, which keeps its clip regardless, so it wouldn't cover copyWithZone: the way the save-side fill does.

-[NSGradient drawInRect:angle:] saves and restores the graphics state
internally, so a clip set before drawing has to survive the state copy.  Add a
case to clipsaverestore.m that clips a gradient to an oval and checks the centre
is painted while a bounding-box corner stays white, the scenario reported in
gnustep/libs-gui#228.
@rmottola

Copy link
Copy Markdown
Member

I will test this trying out real-world usage of applications with your patch applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

saveGraphicsState erases clipping path in Cairo

3 participants