1296657 - imgFrame doesn't care enough about what it's drawing to

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Bas Schouten (:bas.schouten)]

It's better if it does care.

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1296657: Attempt to create an mOptSurface of the correct type for the DT we're drawing to.

Review commit: https://reviewboard.mozilla.org/r/72948/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/72948/

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8782946 [details]
Bug 1296657: Attempt to create an mOptSurface of the correct type for the DT we're drawing to.

Seth's been looking at this code lately so might as well have him look too.

Comment 3

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8782946 [details]
Bug 1296657: Attempt to create an mOptSurface of the correct type for the DT we're drawing to.

https://reviewboard.mozilla.org/r/72948/#review71248

Seems reasonable. feedback+. I think Seth has been changing this lately so he might know of any additional constraints. If he doesn't get back to you in a reasonable time I can take a closer look.

Comment 4

[Jared Wein [:jaws] (please needinfo? me)]

Timothy, do you think you can review this?

Comment 5

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8782946 [details]
Bug 1296657: Attempt to create an mOptSurface of the correct type for the DT we're drawing to.

https://reviewboard.mozilla.org/r/72948/#review75658

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8782946 [details]
Bug 1296657: Attempt to create an mOptSurface of the correct type for the DT we're drawing to.

https://reviewboard.mozilla.org/r/72948/#review76768

::: image/imgFrame.cpp:399
(Diff revision 1)
>  nsresult
> -imgFrame::Optimize()
> +imgFrame::Optimize(DrawTarget* aTarget)
>  {
>    MOZ_ASSERT(NS_IsMainThread());
>    mMonitor.AssertCurrentThreadOwns();
> -  MOZ_ASSERT(mLockCount == 1,
> +  

Why this change?

Comment 7

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)
> Comment on attachment 8782946 [details]
> Bug 1296657: Attempt to create an mOptSurface of the correct type for the DT
> we're drawing to.
> 
> https://reviewboard.mozilla.org/r/72948/#review76768
> 
> ::: image/imgFrame.cpp:399
> (Diff revision 1)
> >  nsresult
> > -imgFrame::Optimize()
> > +imgFrame::Optimize(DrawTarget* aTarget)
> >  {
> >    MOZ_ASSERT(NS_IsMainThread());
> >    mMonitor.AssertCurrentThreadOwns();
> > -  MOZ_ASSERT(mLockCount == 1,
> > +  
> 
> Why this change?

Because we no longer call it in the stage where we are holding the lock.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d378a118900
Attempt to create an mOptSurface of the correct type for the DT we're drawing to. r=tnikkel

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3d378a118900