Closed Bug 888530 Opened 11 years ago Closed 11 years ago

unnecessary memset in ISurfaceAllocator::AllocSurfaceDescriptor

Categories

(Core :: Graphics: Layers, defect)

x86_64
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: gal, Unassigned)

References

()

Details

Attachments

(2 files)

gal: do we rely on surfaces to be clean?
[7:36pm] gal: because I bet you gralloc doesn't guarantee that
[7:36pm] gal: and I didn't see a memset there
[7:37pm] mattwoodrow: OSX shmem was guaranteeing that
[7:37pm] mattwoodrow: and we had failures if I got rid of it
[7:37pm] gal: ok, most the time there won't be alpha
[7:37pm] gal: so this is good enough for me
[7:38pm] mattwoodrow: Also, we clear the surface in ContentClientIncremental::BeginPaintBuffer
[7:38pm] mattwoodrow: so i'm not why the memset would be needed at all
[7:38pm] mattwoodrow: debugging fail at the time maybe
[7:38pm] bbondy left the chat room. (Quit: Going offline, see ya! (www.adiirc.com))
[7:39pm] gal: lets do 2 patches, one to yank the memset
[7:39pm] gal: another to reuse the buffer
[7:39pm] gal: we can try them separately
[7:39pm] gal: or I can do if (aContent != gfaSurface::CONTENT_COLOR) {
[7:39pm] gal: dealers choice
[7:40pm] mattwoodrow: try yanking it entirely
Blocks: 885655
No longer depends on: 885655
Attached patch remove memsetSplinter Review
Attachment #769258 - Flags: review?(matt.woodrow)
Attachment #769258 - Flags: review?(matt.woodrow) → review+
Blocks: 888562
No longer blocks: 888562
Attached patch Remove memset v2Splinter Review
Now passes all tests.
Attachment #772874 - Flags: review?(jmuizelaar)
Attachment #772874 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/c32dc74cd74a
https://hg.mozilla.org/mozilla-central/rev/ef78caaf7301
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Nice work with the mac fix.
You need to log in before you can comment on or make changes to this bug.