Skip to content
This repository has been archived by the owner. It is now read-only.

Add support for surface pattern. #156

Merged
merged 2 commits into from May 15, 2015
Merged

Add support for surface pattern. #156

merged 2 commits into from May 15, 2015

Conversation

@hyowon
Copy link
Contributor

hyowon commented May 14, 2015

I'm interested in implementing canvas features for servo.
SurfacePattern seems to be needed to support pattern fill for canvas.

@jdm

This comment has been minimized.

Copy link

jdm commented on src/azure_hl.rs in 00958cc May 14, 2015

This clone doesn't look safe; we'll have a dangling pointer if the clone outlives the original.

@pcwalton
Copy link
Contributor

pcwalton commented May 14, 2015

This will also be useful for making our tiled background painting code less hilariously slow.

@hyowon
Copy link
Contributor Author

hyowon commented May 15, 2015

@jdm All patterns are cloneable now.

#[derive(Clone)]
pub enum Pattern {
Color(ColorPattern),
LinearGradient(LinearGradientPattern),
RadialGradient(RadialGradientPattern),
Surface(SurfacePattern),
}

I'm not sure whether we can remove clone from all patterns or not.
But CanvasPaintState having Patterns needs clone.

#[derive(Clone)]
struct CanvasPaintState<'a> {
draw_options: DrawOptions,
fill_style: Pattern,
stroke_style: Pattern,
stroke_opts: StrokeOptions<'a>,
/// The current 2D transform matrix.
transform: Matrix2D,
}

Is it difficult to keep on using clone while avoiding a dangling pointer?
I'm a newbie to rust and servo. Please advice me further. Thank you for your review.

@pcwalton Interesting. I'll also see the tiled background painting code.

@jdm
Copy link
Member

jdm commented May 15, 2015

Yeah, I just noticed that the other pattern types derive Clone as well, so they have the same problem. The way to fix it here is to add a C API to clone an AzSurfacePatternRef that casts it to gfx::SourcePattern and returns new SurfacePattern(sourcePattern->mSurface, sourcePattern->mExtendMode, sourcePatter->mFilter, sourcePattern->mMatrix, sourcePattern->mSamplingRect). We should do the same for the other pattern types, and manually implement the Clone trait instead of deriving it. Does that make sense?

@hyowon
Copy link
Contributor Author

hyowon commented May 15, 2015

That makes great sense. Thank you for your kind guidance. Please review additional patch.

@hyowon hyowon force-pushed the hyowon:master branch from 7d36a19 to a7bab6c May 15, 2015
jdm added a commit that referenced this pull request May 15, 2015
Add support for surface pattern.
@jdm jdm merged commit ecd3e60 into servo:master May 15, 2015
@jdm
Copy link
Member

jdm commented May 15, 2015

Looks great!

@hyowon
Copy link
Contributor Author

hyowon commented Jun 1, 2015

cc @yichoi

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.