Closed Bug 685767 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla11

People

(Reporter: joe, Assigned: joe)

References

Details

Attachments

(2 files)

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: nobody → joe
Attachment #559364 - Flags: review?(matt.woodrow) → review+
https://hg.mozilla.org/mozilla-central/rev/6ae6d3beeaf4
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Depends on: 702683
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
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Stupid bugzilla - was only adding myself and if closed - Reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
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+
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
Duplicate of this bug: 702683
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?
(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
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
(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.
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?
Blocks: 739969
No longer blocks: 739969
Depends on: 989885
You need to log in before you can comment on or make changes to this bug.