We depend on undefined clipping semantics of cairo

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
It looks like we currently depend on the clip on a surface persisting during the use of native apis. Cairo trunk has changed the semantics here and thus breaks us.
(Assignee)

Updated

8 years ago
Blocks: 542605
(Assignee)

Comment 1

8 years ago
Basically it looks like UpdateSurfaceClip() no longer works.
(Assignee)

Updated

8 years ago
OS: Mac OS X → All
Hardware: x86 → All
Version: 1.9.2 Branch → Trunk
(Assignee)

Comment 2

8 years ago
I previously tried to fix UpdateSurfaceClip by drawing a 1x1 transparent pixel. This worked in many cases but was not sufficient for ensuring that the clip made it to the destination. I'm not sure what the best solution is here...
Perhaps we should add new cairo API here. Perhaps it should be backend-specific.
(Assignee)

Comment 4

8 years ago
(In reply to comment #3)
> Perhaps we should add new cairo API here. Perhaps it should be
> backend-specific.

I thought about adding backend-specific API but it doesn't really fit in nice, because the clip is associated with a cairo_t which can have a many-to-one relationship with the surface. i.e. the surface/backend doesn't really have any way of knowing what the clip should be.

The best option I can think of right now is to add api get the clip out of a cairo_t and then apply that to the destination surface ourselves.
We have cairo_copy_clip_rectangle_list, but that doesn't work with general paths.

Having to write our own per-backend code to apply the cairo clip to a native context sounds pretty bad since it's duplicating code already in cairo.

How about backend-specific API that take a cairo_t? E.g.
HDC cairo_win32_get_dc_with_clip (cairo_t *cr)
where if the current target of the cr is a Win32 surface, we return the HDC and we ensure that the cairo clip has been set on that DC?

Similarly, cairo_quartz_get_cg_context_with_clip, etc.
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> How about backend-specific API that take a cairo_t? E.g.
> HDC cairo_win32_get_dc_with_clip (cairo_t *cr)
> where if the current target of the cr is a Win32 surface, we return the HDC and
> we ensure that the cairo clip has been set on that DC?
> 
> Similarly, cairo_quartz_get_cg_context_with_clip, etc.

That's not a bad idea. One concern I have is that the semantics of those to functions are different. i.e. win32 only supports rectangular clips. Maybe it's not worth worrying about.
(In reply to comment #6)
> (In reply to comment #5)
> > How about backend-specific API that take a cairo_t? E.g.
> > HDC cairo_win32_get_dc_with_clip (cairo_t *cr)
> > where if the current target of the cr is a Win32 surface, we return the HDC and
> > we ensure that the cairo clip has been set on that DC?
> > 
> > Similarly, cairo_quartz_get_cg_context_with_clip, etc.
> 
> That's not a bad idea. One concern I have is that the semantics of those to
> functions are different. i.e. win32 only supports rectangular clips. Maybe it's
> not worth worrying about.

Do not underestimate the amount of non-rectangular clips we get, for example a div with rounded borders would produce one if I'm not mistaking.
I think we could define cairo_win32_get_dc_with_clip to return NULL if the clip cannot be set on the HDC --- i.e. it's not a single rectangle. That will work fine for us.
(Assignee)

Comment 9

8 years ago
(In reply to comment #7)
> Do not underestimate the amount of non-rectangular clips we get, for example a
> div with rounded borders would produce one if I'm not mistaking.

We currently rely on only clipping the clip region and not the clip path on win32. If we want to change this we can do that after the cairo update. I wasn't able to come across any cases where we actually want to clip native stuff to non-rectangular paths. For example, overflow:hidden on a div with border-radius doesn't clip to rounded rectangle (though perhaps it should as webkit does...)
(Assignee)

Comment 10

8 years ago
Created attachment 439365 [details] [diff] [review]
First cut at what this could look like

Here's a copy of what this could look like. Anyone have any thoughts?
Assignee: nobody → jmuizelaar
Attachment #439365 - Flags: feedback?(chris)
-cairo_quartz_surface_get_cg_context (cairo_surface_t *surface);

Probably don't want to remove this.

I'm not sure why the Win32 printing code is different from the main Win32 code.

Otherwise looks good.
(Assignee)

Comment 12

8 years ago
Created attachment 440570 [details] [diff] [review]
Completed version
Attachment #439365 - Attachment is obsolete: true
Attachment #440570 - Flags: review?(vladimir)
Attachment #440570 - Flags: feedback?(chris)
Attachment #439365 - Flags: feedback?(chris)
Comment on attachment 440570 [details] [diff] [review]
Completed version

looks fine to me, but boy does it need documentation in cairo :)
Attachment #440570 - Flags: review?(vladimir) → review+
(Assignee)

Updated

8 years ago
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.