Closed Bug 1091321 Opened 5 years ago Closed 5 years ago

Convert all the SVG functions that take an nsRenderingContext to take a gfxContext instead

Categories

(Core :: SVG, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jwatt, Assigned: jwatt)

References

Details

Attachments

(7 files, 1 obsolete file)

No description provided.
Attachment #8513946 - Flags: review?(longsonr)
Attachment #8513947 - Flags: review?(longsonr)
Attachment #8513948 - Flags: review?(longsonr)
Attached patch PaintSVGSplinter Review
Attachment #8513949 - Flags: review?(longsonr)
Attached patch PaintMarkSplinter Review
Attachment #8513950 - Flags: review?(longsonr)
Attached patch PaintFramesWithEffects (obsolete) — Splinter Review
Attachment #8513951 - Flags: review?(longsonr)
Attachment #8513951 - Attachment is obsolete: true
Attachment #8513951 - Flags: review?(longsonr)
Attachment #8513952 - Flags: review?(longsonr)
Attachment #8513953 - Flags: review?(longsonr)
Comment on attachment 8513946 [details] [diff] [review]
ApplyClipOrPaintClipMask

Review of attachment 8513946 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxContext.cpp
@@ +683,5 @@
> +  AzureState::PushedClip clip = { aPath, Rect(), mTransform };
> +  CurrentState().pushedClips.AppendElement(clip);
> +}
> +
> +void

Duplicates the second half of gfxContext::Clip(). Perhaps gfxContext::Clip() should call this. And the rect clip above should be called by gfxContext::Clip() rather than calling it for consistency.

Also while you're here you might change the rect clip argument above to be aRect.

::: gfx/thebes/gfxContext.h
@@ +504,5 @@
>       * Any current path will be destroyed by these functions!
>       */
>      void Clip(const Rect& rect);
>      void Clip(const gfxRect& rect); // will clip to a rect
> +    void Clip(Path* aPath);

aPath should be const
Attachment #8513946 - Flags: review?(longsonr) → review+
Attachment #8513947 - Flags: review?(longsonr) → review+
Attachment #8513948 - Flags: review?(longsonr) → review+
Attachment #8513949 - Flags: review?(longsonr) → review+
Attachment #8513950 - Flags: review?(longsonr) → review+
Attachment #8513952 - Flags: review?(longsonr) → review+
Attachment #8513953 - Flags: review?(longsonr) → review+
(In reply to Robert Longson from comment #9)
> ::: gfx/thebes/gfxContext.cpp
> @@ +683,5 @@
> > +  AzureState::PushedClip clip = { aPath, Rect(), mTransform };
> > +  CurrentState().pushedClips.AppendElement(clip);
> > +}
> > +
> > +void
> 
> Duplicates the second half of gfxContext::Clip(). Perhaps gfxContext::Clip()
> should call this. And the rect clip above should be called by
> gfxContext::Clip() rather than calling it for consistency.
> 
> Also while you're here you might change the rect clip argument above to be
> aRect.

This code originally came from bug 1090614 but I moved it here because I thought I'd land this first. That didn't happen though because this bug is still blocked on review in bug 651021. When I landed 1090614 I forgot to move the Clip changes back to the patches there, which broke the tree. I then landed them in a hurry to fix the bustage without addressing the comments here. Since we hope to have killed off gfxContext in the next 6-12 months I don't think we care about keeping it really neat and tidy though.

> ::: gfx/thebes/gfxContext.h
> @@ +504,5 @@
> >       * Any current path will be destroyed by these functions!
> >       */
> >      void Clip(const Rect& rect);
> >      void Clip(const gfxRect& rect); // will clip to a rect
> > +    void Clip(Path* aPath);
> 
> aPath should be const

It can't be, since the state stack keeps pointer s to non-const. Besides that Path objects are immutable anyway.
You need to log in before you can comment on or make changes to this bug.