Factor blur internals out of gfxBlur into a standalone class in gfx/2d

RESOLVED FIXED in mozilla11

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: Joe Drew (not getting mail), Assigned: Joe Drew (not getting mail))

Tracking

Trunk
mozilla11
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

6 years ago
Created attachment 559364 [details] [diff] [review]
move innards of gfxAlphaBoxBlur to mozilla::gfx::AlphaBoxBlur

In order to implement shadows in the Cairo Azure backend, we need to have Gaussian blur support. The easiest way to do that is to to make the innards of gfxAlphaBoxBlur into their own class that uses it.

This patch does that, creating mozilla::gfx::AlphaBoxBlur.
Attachment #559364 - Flags: review?(matt.woodrow)
(Assignee)

Updated

6 years ago
Assignee: nobody → joe
Attachment #559364 - Flags: review?(matt.woodrow) → review+

Comment 1

6 years ago
https://hg.mozilla.org/mozilla-central/rev/6ae6d3beeaf4
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Depends on: 702683

Updated

6 years ago
Depends on: 703125
Backed out for leaking (see Bug 703125).

https://hg.mozilla.org/mozilla-central/rev/b62e6ee5ba9b
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
With next psuh don't forget bug 702683 patch
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
Stupid bugzilla - was only adding myself and if closed - Reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 5

6 years ago
Created attachment 575377 [details] [diff] [review]
work around compiler bug

It seems that MSVC has a compiler bug that's triggered by AlphaBoxBlur being in the namespace mozilla::gfx. Due to link-time optimization, instead of calling an nsAutoPtr destructor that calls |delete ptr|, it calls a destructor that calls |delete[] ptr|, which doesn't call AlphaBoxBlur's destructor at all.

If we move AlphaBoxBlur to the global namespace, the bug doesn't manifest itself. And since we don't want to do that, if we change from nsAutoPtr to a raw ptr, and call delete on it manually in gfxBlur's destructor, this bug also disappears.

This is an interdiff, on top of the previous patch.
Attachment #575377 - Flags: review?(matt.woodrow)
Attachment #575377 - Flags: review?(matt.woodrow) → review+
(Assignee)

Comment 6

6 years ago
I've just pushed the combined patches on this bug, as well as the fix to bug 702683, to inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/5549f4202af8
(Assignee)

Updated

6 years ago
Duplicate of this bug: 702683

Updated

6 years ago
OS: Mac OS X → All
Hardware: x86 → All
Joe, can you file a separate bug on investigating if there are any other uses of nsAutoPtr being miscompiled by MSVC?

Comment 9

6 years ago
(In reply to Joe Drew (:JOEDREW!) from comment #6)
> I've just pushed the combined patches on this bug, as well as the fix to bug
> 702683, to inbound:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/5549f4202af8

https://hg.mozilla.org/mozilla-central/rev/5549f4202af8

(But don't forget comment 8 :-))
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

6 years ago
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #8)
> Joe, can you file a separate bug on investigating if there are any other
> uses of nsAutoPtr being miscompiled by MSVC?

This is (mentored) bug 704304.
Depends on: 721663

Comment 11

5 years ago
I think fixed-point division introduced by Bug 633627 has been returned again to integer division by the patch of this bug. Isn't this a regression?
(In reply to Tetsuro Kato (Tete) from comment #11)
> I think fixed-point division introduced by Bug 633627 has been returned
> again to integer division by the patch of this bug. Isn't this a regression?

Yes it is, can you file a new bug for it?

Updated

5 years ago
Blocks: 739969

Updated

5 years ago
No longer blocks: 739969

Comment 13

5 years ago
Fired a new bug.
https://bugzilla.mozilla.org/show_bug.cgi?id=739969

Updated

3 years ago
Depends on: 989885
You need to log in before you can comment on or make changes to this bug.