Stop using gfxImageSurface in gfxBlur

RESOLVED FIXED in mozilla33

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jwatt, Assigned: jwatt)

Tracking

Trunk
mozilla33
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

3.58 KB, patch
mattwoodrow
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
Stop using gfxImageSurface in gfxBlur
(Assignee)

Comment 1

5 years ago
Created attachment 8440317 [details] [diff] [review]
patch

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-
(Assignee)

Comment 4

5 years ago
Should CreateDrawTargetForData maybe fall back to using BackendType::CAIRO in that case?
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 5

5 years ago
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)
(Assignee)

Comment 7

5 years ago
(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. :)
(Assignee)

Comment 8

5 years ago
Created attachment 8446162 [details] [diff] [review]
patch

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
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.