Closed Bug 1025537 Opened 6 years ago Closed 6 years ago

Stop using gfxImageSurface in gfxBlur

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: jwatt, Assigned: jwatt)

Details

Attachments

(1 file, 1 obsolete file)

Stop using gfxImageSurface in gfxBlur
Attached patch patch (obsolete) — Splinter Review
This code seems a bit pointless.
Attachment #8440317 - Flags: review?(mstange)
Comment on attachment 8440317 [details] [diff] [review]
patch

Matt added this code, he'll know why it looks the way it does.
Attachment #8440317 - Flags: review?(mstange) → review?(matt.woodrow)
Comment on attachment 8440317 [details] [diff] [review]
patch

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

::: gfx/thebes/gfxBlur.cpp
@@ +68,5 @@
>          gfxPlatform::GetPlatform()->CreateDrawTargetForData(mData, size,
>                                                              mBlur->GetStride(),
>                                                              SurfaceFormat::A8);
>      if (!dt) {
> +        return nullptr;

CreateDrawTargetForData will return nullptr if the content backend is d2d, so we need to have a fallback.
Attachment #8440317 - Flags: review?(matt.woodrow) → review-
Should CreateDrawTargetForData maybe fall back to using BackendType::CAIRO in that case?
Flags: needinfo?(matt.woodrow)
And do you have any idea why gfxPlatform::CreateDrawTargetForData creates a gfxImageSurface?
(In reply to Jonathan Watt [:jwatt] from comment #4)
> Should CreateDrawTargetForData maybe fall back to using BackendType::CAIRO
> in that case?

Yeah that's probably ok for the gfxPlatform version, not the Moz2D one.

(In reply to Jonathan Watt [:jwatt] from comment #5)
> And do you have any idea why gfxPlatform::CreateDrawTargetForData creates a
> gfxImageSurface?

None at all. hg history might reveal something.
Flags: needinfo?(matt.woodrow)
(In reply to Matt Woodrow (:mattwoodrow) from comment #6)
> None at all. hg history might reveal something.

I already checked that, but it didn't. I'll try replacing it and see if the sky falls in. :)
Attached patch patchSplinter Review
This seems to pass Try.
Attachment #8440317 - Attachment is obsolete: true
Attachment #8446162 - Flags: review?(matt.woodrow)
Attachment #8446162 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/cd59e16a415d
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.