Use normal memory instead of Shmem when passing textures between threads on the same process

RESOLVED FIXED in mozilla24

Status

()

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mattwoodrow, Unassigned)

Tracking

unspecified
mozilla24
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

6 years ago
Created attachment 752086 [details] [diff] [review]
Use raw memory

We spend significant amounts of time allocating Shmem with OMTC, and we are only sharing memory between threads in the same process.

This lets us just normal memory instead in this case, and makes everything happier.
Attachment #752086 - Flags: review?(bas)
(Reporter)

Comment 1

6 years ago
Created attachment 752090 [details] [diff] [review]
Use raw memory v2

Forgot to qref and include a memory leak fix
Attachment #752086 - Attachment is obsolete: true
Attachment #752086 - Flags: review?(bas)
Attachment #752090 - Flags: review?(bas)
Comment on attachment 752090 [details] [diff] [review]
Use raw memory v2

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

::: gfx/layers/ipc/ISurfaceAllocator.cpp
@@ +90,5 @@
> +    gfxImageFormat format = aContent == gfxASurface::CONTENT_COLOR_ALPHA ?
> +                            gfxASurface::ImageFormatARGB32 :
> +                            gfxASurface::ImageFormatRGB24;
> +    int32_t stride = gfxASurface::FormatStrideForWidth(format, aSize.width);
> +    unsigned char *data = new unsigned char[stride * aSize.height];

nit: For consistency replace unsigned char by uint8_t.

::: gfx/layers/ipc/ShadowLayerUtilsMac.cpp
@@ +38,5 @@
> +    const MemoryImage& image = aSurface.get_MemoryImage();
> +    gfxASurface::gfxImageFormat format
> +      = static_cast<gfxASurface::gfxImageFormat>(image.format());
> +
> +    nsRefPtr<gfxASurface> surf =

Hrm, I wonder if we always want a Quartz surface here or if we want the ability to do something else. For now this should be fine though.
Attachment #752090 - Flags: review?(bas) → review+
(Reporter)

Comment 4

6 years ago
Backed out for causing b2g reftest failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/09ff5b55b27a
(Reporter)

Comment 5

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb160edaec7c

Re-landed with the content -> format conversion implemented correctly.
https://hg.mozilla.org/mozilla-central/rev/fb160edaec7c
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
sounds like this fix breaking OMTC SW rendering... 

Assertion failure: aDescriptorType == SurfaceDescriptor::TShmem (We can only support Shmem currently), at mozilla-central/gfx/layers/basic/BasicCompositor.cpp:74
You need to log in before you can comment on or make changes to this bug.