Incorrect alpha value for BGRX surface on Windows with Skia content

RESOLVED FIXED in Firefox 51

Status

()

Core
Graphics
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mchang, Assigned: mchang)

Tracking

unspecified
mozilla51
x86
Windows
Points:
---

Firefox Tracking Flags

(firefox51 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

2 years ago
13:02:09  WARNING -  PROCESS-CRASH | Main app process exited normally | application crashed [@ mozilla::gfx::VerifyRGBXFormat]
13:02:09     INFO -  Crash dump filename: c:\users\cltbld\appdata\local\temp\tmphdxmfk.mozrunner\minidumps\156b6aa8-4924-490c-8c9e-09abe1ad6114.dmp
13:02:09     INFO -  Operating system: Windows NT
13:02:09     INFO -                    6.1.7601 Service Pack 1
13:02:09     INFO -  CPU: x86
13:02:09     INFO -       GenuineIntel family 6 model 30 stepping 5
13:02:09     INFO -       8 CPUs
13:02:09     INFO -  Crash reason:  EXCEPTION_BREAKPOINT
13:02:09     INFO -  Crash address: 0x5cefa92d
13:02:09     INFO -  Assertion: Unknown assertion type 0x00000000
13:02:09     INFO -  Process uptime: 13 seconds
13:02:09     INFO -  Thread 0 (crashed)
13:02:09     INFO -   0  xul.dll!mozilla::gfx::VerifyRGBXFormat [DrawTargetSkia.cpp:1df72edea799 : 122 + 0x20]
13:02:09     INFO -      eip = 0x5cefa92d   esp = 0x0015f0ec   ebp = 0x0015f0f4   ebx = 0x0ebc18c0
13:02:09     INFO -      esi = 0x0000007a   edi = 0x06630073   eax = 0x5f9fdb6c   ecx = 0x6909705d
13:02:09     INFO -      edx = 0x76e770b4   efl = 0x00000202
13:02:09     INFO -      Found by: given as instruction pointer in context
13:02:09     INFO -   1  xul.dll!mozilla::gfx::DrawTargetSkia::OptimizeSourceSurface(mozilla::gfx::SourceSurface *) [DrawTargetSkia.cpp:1df72edea799 : 1409 + 0x56]
13:02:09     INFO -      eip = 0x5cef9804   esp = 0x0015f0fc   ebp = 0x0015f178
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   2  xul.dll!gfxPlatform::GetSourceSurfaceForSurface(mozilla::gfx::DrawTarget *,gfxASurface *) [gfxPlatform.cpp:1df72edea799 : 1066 + 0xf]
13:02:09     INFO -      eip = 0x5d07feac   esp = 0x0015f180   ebp = 0x0015f1b4
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   3  xul.dll!mozilla::plugins::PluginInstanceParent::RecvShow(_NPRect const &,mozilla::plugins::SurfaceDescriptor const &,mozilla::plugins::SurfaceDescriptor *) [PluginInstanceParent.cpp:1df72edea799 : 957 + 0x11]
13:02:09     INFO -      eip = 0x5df04e0f   esp = 0x0015f1bc   ebp = 0x0015f288
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   4  xul.dll!std::_Hash<stdext::_Hmap_traits<int,mozilla::ipc::IProtocol *,stdext::hash_compare<int,std::less<int> >,std::allocator<std::pair<int const ,mozilla::ipc::IProtocol *> >,0> >::lower_bound(int const &) [xhash:1df72edea799 : 607 + 0x6]
13:02:09     INFO -      eip = 0x5c9caf8f   esp = 0x0015f1e4   ebp = 0x0015f288
13:02:09     INFO -      Found by: stack scanning
13:02:09     INFO -   5  xul.dll!mozilla::plugins::PPluginInstanceParent::OnMessageReceived(IPC::Message const &,IPC::Message * &) [PPluginInstanceParent.cpp:1df72edea799 : 2284 + 0x9]
13:02:09     INFO -      eip = 0x5cac7d9d   esp = 0x0015f290   ebp = 0x0015f32c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   6  xul.dll!mozilla::plugins::PPluginModuleParent::OnMessageReceived(IPC::Message const &,IPC::Message * &) [PPluginModuleParent.cpp:1df72edea799 : 1560 + 0x9]
13:02:09     INFO -      eip = 0x5caddd0b   esp = 0x0015f334   ebp = 0x0015f36c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   7  xul.dll!mozilla::ipc::MessageChannel::DispatchSyncMessage(IPC::Message const &,IPC::Message * &) [MessageChannel.cpp:1df72edea799 : 1632 + 0x11]
13:02:09     INFO -      eip = 0x5c97d490   esp = 0x0015f374   ebp = 0x0015f390
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   8  xul.dll!mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message &&) [MessageChannel.cpp:1df72edea799 : 1595 + 0x16]
13:02:09     INFO -      eip = 0x5c97d1f9   esp = 0x0015f398   ebp = 0x0015f3f4
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -   9  xul.dll!mozilla::ipc::MessageChannel::OnMaybeDequeueOne() [MessageChannel.cpp:1df72edea799 : 1566 + 0xb]
13:02:09     INFO -      eip = 0x5c9808e8   esp = 0x0015f3fc   ebp = 0x0015f46c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  10  xul.dll!mozilla::detail::RunnableMethodImpl<bool ( mozilla::ipc::MessageChannel::*)(void),0,1>::Run() [nsThreadUtils.h:1df72edea799 : 764 + 0x3]
13:02:09     INFO -      eip = 0x5c9837d6   esp = 0x0015f474   ebp = 0x0015f474
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  11  xul.dll!mozilla::ipc::MessageChannel::DequeueTask::Run() [MessageChannel.h:1df72edea799 : 569 + 0x13]
13:02:09     INFO -      eip = 0x5c983b05   esp = 0x0015f47c   ebp = 0x0015f480
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  12  xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:1df72edea799 : 1068 + 0xe]
13:02:09     INFO -      eip = 0x5c5ba5c9   esp = 0x0015f488   ebp = 0x0015f4f4
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  13  xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:1df72edea799 : 290 + 0xd]
13:02:09     INFO -      eip = 0x5c5e1f81   esp = 0x0015f4fc   ebp = 0x0015f508
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  14  xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [MessagePump.cpp:1df72edea799 : 100 + 0x8]
13:02:09     INFO -      eip = 0x5c983e71   esp = 0x0015f510   ebp = 0x0015f534
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  15  xul.dll!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate *) [MessagePump.cpp:1df72edea799 : 317 + 0x8]
13:02:09     INFO -      eip = 0x5c983f9d   esp = 0x0015f53c   ebp = 0x0015f54c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  16  xul.dll!MessageLoop::RunInternal() [message_loop.cc:1df72edea799 : 232 + 0xf]
13:02:09     INFO -      eip = 0x5c961524   esp = 0x0015f554   ebp = 0x0015f56c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  17  xul.dll!MessageLoop::RunHandler() [message_loop.cc:1df72edea799 : 225 + 0x5]
13:02:09     INFO -      eip = 0x5c9614dc   esp = 0x0015f574   ebp = 0x0015f5a0
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  18  xul.dll!MessageLoop::Run() [message_loop.cc:1df72edea799 : 205 + 0x7]
13:02:09     INFO -      eip = 0x5c961227   esp = 0x0015f5a8   ebp = 0x0015f5c0
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  19  xul.dll!nsBaseAppShell::Run() [nsBaseAppShell.cpp:1df72edea799 : 156 + 0xc]
13:02:09     INFO -      eip = 0x5e18720e   esp = 0x0015f5c8   ebp = 0x0015f5d0
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  20  xul.dll!nsAppShell::Run() [nsAppShell.cpp:1df72edea799 : 262 + 0x8]
13:02:09     INFO -      eip = 0x5e1d76c2   esp = 0x0015f5d8   ebp = 0x0015f5e0
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  21  xul.dll!XRE_RunAppShell [nsEmbedFunctions.cpp:1df72edea799 : 851 + 0xe]
13:02:09     INFO -      eip = 0x5e95da6d   esp = 0x0015f5e8   ebp = 0x0015f5f4
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  22  xul.dll!mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate *) [MessagePump.cpp:1df72edea799 : 285 + 0x5]
13:02:09     INFO -      eip = 0x5c983ed4   esp = 0x0015f5fc   ebp = 0x0015f608
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  23  xul.dll!MessageLoop::RunInternal() [message_loop.cc:1df72edea799 : 232 + 0xf]
13:02:09     INFO -      eip = 0x5c961524   esp = 0x0015f610   ebp = 0x0015f628
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  24  xul.dll!MessageLoop::RunHandler() [message_loop.cc:1df72edea799 : 225 + 0x5]
13:02:09     INFO -      eip = 0x5c9614dc   esp = 0x0015f630   ebp = 0x0015f65c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  25  xul.dll!MessageLoop::Run() [message_loop.cc:1df72edea799 : 205 + 0x7]
13:02:09     INFO -      eip = 0x5c961227   esp = 0x0015f664   ebp = 0x0015f67c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  26  xul.dll!XRE_InitChildProcess [nsEmbedFunctions.cpp:1df72edea799 : 681 + 0xb]
13:02:09     INFO -      eip = 0x5e95d609   esp = 0x0015f684   ebp = 0x0015f7f8
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  27  firefox.exe!content_process_main(int,char * * const) [plugin-container.cpp:1df72edea799 : 224 + 0xc]
13:02:09     INFO -      eip = 0x001c182f   esp = 0x0015f800   ebp = 0x0015f824
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  28  firefox.exe!NS_internal_main(int,char * *,char * *) [nsBrowserApp.cpp:1df72edea799 : 357 + 0x9]
13:02:09     INFO -      eip = 0x001c15b2   esp = 0x0015f82c   ebp = 0x0015f85c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  29  firefox.exe!wmain [nsWindowsWMain.cpp:1df72edea799 : 115 + 0xf]
13:02:09     INFO -      eip = 0x001c201e   esp = 0x0015f864   ebp = 0x0015f89c
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  30  firefox.exe!__scrt_common_main_seh [exe_common.inl : 255 + 0x1d]
13:02:09     INFO -      eip = 0x001f19e5   esp = 0x0015f8a4   ebp = 0x0015f8e8
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  31  kernel32.dll!BaseThreadInitThunk + 0x12
13:02:09     INFO -      eip = 0x75323c45   esp = 0x0015f8f0   ebp = 0x0015f8f4
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  32  ntdll.dll!__RtlUserThreadStart + 0x27
13:02:09     INFO -      eip = 0x76e937f5   esp = 0x0015f8fc   ebp = 0x0015f934
13:02:09     INFO -      Found by: call frame info
13:02:09     INFO -  33  ntdll.dll!_RtlUserThreadStart + 0x1b
13:02:09     INFO -      eip = 0x76e937c8   esp = 0x0015f93c   ebp = 0x0015f94c
13:02:09     INFO -      Found by: call frame info

From: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1df72edea799&selectedJob=24536342
(Assignee)

Updated

2 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8775824 [details] [diff] [review]
Always set SharedDIBSurfaces to RGBA surface format

Or we could memset here:

https://hg.mozilla.org/try/rev/d17c8fdd1395831063cc29e65de736f2bd32b197
Attachment #8775824 - Flags: review?(lsalzman)
Comment on attachment 8775824 [details] [diff] [review]
Always set SharedDIBSurfaces to RGBA surface format

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

This is not a great thing to do. I don't think we should be using SharedDIBSurfaces with Skia
Attachment #8775824 - Flags: review?(lsalzman) → review-
(Assignee)

Comment 3

2 years ago
Created attachment 8776116 [details] [diff] [review]
Use SharedImage for plugin surface if Skia backend

Since this was a plugin, we had to do a bunch of stuff to detect if Skia is actually being used. I added the backend type over IPDL to send the information from the parent process to the plugin process since gfxPrefs can't be accessed on the plugin process w/o initializing XPCOM.

The rest of the patch stops using DIBSurfaces if we have a Skia backend. Finally, if we do use Skia and use an gfxImageSurface, we memset it to 0xFF for opaque surfaces.
Attachment #8775824 - Attachment is obsolete: true
Attachment #8776116 - Flags: review?(jmuizelaar)
Comment on attachment 8776116 [details] [diff] [review]
Use SharedImage for plugin surface if Skia backend

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

I think the correct way to fix this is to actually set the alpha byte on input into Skia from the plugin world. GDI can totally produce BGRX data that actually depends on us ignoring the alpha byte.
Attachment #8776116 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 5

2 years ago
Created attachment 8778003 [details] [diff] [review]
Write alpha channel for plugin content
Attachment #8776116 - Attachment is obsolete: true
Attachment #8778003 - Flags: review?(jmuizelaar)
Comment on attachment 8778003 [details] [diff] [review]
Write alpha channel for plugin content

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

::: gfx/2d/2D.h
@@ +1036,5 @@
>     * arbitrary SourceSurface type supported by this backend. This may return
>     * aSourceSurface or some other existing surface.
>     */
>    virtual already_AddRefed<SourceSurface> OptimizeSourceSurface(SourceSurface *aSurface) const = 0;
> +  virtual already_AddRefed<SourceSurface> OptimizeSourceSurfaceForPlugin(SourceSurface *aSurface) const {

OptimizeSourceSurfaceForUnknownAlpha and maybe adjust the other places.
Attachment #8778003 - Flags: review?(jmuizelaar) → review+

Comment 9

2 years ago
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/731ede5ac96a
Write alpha values for plugin surfaces when using the Skia backend. r=jrmuizel

Comment 10

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/731ede5ac96a
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.