Open Bug 1716707 Opened 3 years ago Updated 3 months ago

[s390x] Software WebRender does not support big endian

Categories

(Core :: Graphics: WebRender, defect)

Firefox 89
Other
Linux
defect

Tracking

()

UNCONFIRMED

People

(Reporter: msirringhaus, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files, 1 obsolete file)

With the resolution of bug 1701231, colors on big endian platforms are wrong again (if all the Skia patches that roam this bugzilla are applied), as SW WebRender seems to be little endian specific for certain things.

For those who need a workaround on big endian platforms now, I reverted the commit with the following patch:

diff -r 16c46206d0b4 widget/gtk/GfxInfo.cpp
--- a/widget/gtk/GfxInfo.cpp    Tue Jun 15 15:07:55 2021 +0200
+++ b/widget/gtk/GfxInfo.cpp    Wed Jun 16 08:29:14 2021 +0200
@@ -749,12 +749,13 @@
 
     ////////////////////////////////////
     // FEATURE_WEBRENDER_SOFTWARE - ALLOWLIST
+#if MOZ_LITTLE_ENDIAN()
     APPEND_TO_DRIVER_BLOCKLIST(OperatingSystem::Linux, DeviceFamily::All,
                                nsIGfxInfo::FEATURE_WEBRENDER_SOFTWARE,
                                nsIGfxInfo::FEATURE_ALLOW_ALWAYS,
                                DRIVER_COMPARISON_IGNORED, V(0, 0, 0, 0),
                                "FEATURE_ROLLOUT_SOFTWARE_WR", "");
-
+#endif
     ////////////////////////////////////
     // FEATURE_WEBRENDER_COMPOSITOR
     APPEND_TO_DRIVER_BLOCKLIST(

Not sure, if that patch is worth submitting?

Hopefully, we can get WebRender to support big endian at some point, in order to switch to it and finally leave skia behind.

Blocks: big-endian
Regressions: 1701231

Yeah, support for disabling WebRender may be going away as earlier as 92 so it would be best to get started on debugging what the problem is.

Also, FWIW, software webrender was disabled on 89/90 in 89.0.1 in bug 1715895

This is probably caused by SGWL producing BGRA and cairo expecting ARGB on big endian.

Flags: needinfo?(lsalzman)

Sotaro had already investigated doing a BGRA->RGBA conversion for Android. Maybe he has ideas about the best place to handle a similar BGRA->ARGB conversion in RCSWGL and how to do it. Any swizzle here would be acceptable I guess, regardless of performance implications, since big-endian is tier-3 and we have no development hardware to do any actual testing on. It is acceptable for it to merely work at all...

Flags: needinfo?(lsalzman) → needinfo?(sotaro.ikeda.g)

(In reply to msirringhaus from comment #0)

Not sure, if that patch is worth submitting?

Hopefully, we can get WebRender to support big endian at some point, in order to switch to it and finally leave skia behind.

Block list for software WebRender is not used on non-Android platform by Bug 1715132.

Flags: needinfo?(aosmond)
Flags: needinfo?(aosmond)

"Merely working at all" is good enough for me, for now. :-)
Will the ability to disable WebRender still be there in 91 ESR? Otherwise, I have to rework my schedule.

(In reply to Lee Salzman [:lsalzman] from comment #4)

Sotaro had already investigated doing a BGRA->RGBA conversion for Android. Maybe he has ideas about the best place to handle a similar BGRA->ARGB conversion in RCSWGL and how to do it.

It seems that mDT->CopySurface() in RenderCompositorSWGL::CommitMappedBuffer() could be a good place for BGRA->RGBA conversion by using BufferMode::BUFFERED mode.

Flags: needinfo?(sotaro.ikeda.g)

(In reply to msirringhaus from comment #6)

"Merely working at all" is good enough for me, for now. :-)
Will the ability to disable WebRender still be there in 91 ESR? Otherwise, I have to rework my schedule.

Yes, 91 ESR should still be able to disable WebRender. It will be the last ESR to be able to do so. In an ideal world we would uplift patches to it to allow WebRender since that's the rendering path that will be well supported going forward, the Basic at least is mature at this point and doesn't see a lot of changes (besides getting removed in some later release :)).

Severity: -- → S4
Depends on: 1717572

With the new year, I'm picking this up again, to prepare for the next ESR cycle.

State right now:
Images are rendered fine, text is blue, menu-text is broken completely, chrome-icons invisible.

After some discussions on #gfx, I tried using LIBGL_ALWAYS_SOFTWARE=1 + gfx.webrender.all=true, which gives me what can be seen on the attached screenshot:
Images are now wrong (blueish), but text is fine. menu+icons still invisible.

Indicating that text color is likely broken in the SW-GL code.

Attached patch blue-font.patch (obsolete) — Splinter Review

I managed to get a patch that fixes the blue font issue on big endian, but I don't like it much. The location for the swizzle seems odd, but I couldn't find a better place yet, which is why I'm only attaching the patch here for now, instead of submitting it.

This patch will make the navigation elements (back-button, etc.) visible again.
But after a discussion on matrix, I'm now switching gears a little. Instead of trying to find and swizzle all broken things, I'll now try to find one single spot for swizzling all rendering output, before passing it to cairo.
Therefore, this patch is only meant as a fallback-solution, if I fail to do it "the right way".

Ok, so using this new patch in combination with the svg-rendering.patch comes pretty close to 'good enough' for me on s390x.
I'll have to do a full release rebuild tomorrow and test it out a bit more thoroughly, though.

Attachment #9260555 - Attachment is obsolete: true

Ok, after testing today, it seems like most things look normal now on big endian, with these 2 patches. With the notable exception of link-previews on Wikipedia (while hovering over a link), which is just white, and the background color of the About dialog, which is green.

I can confirm that these make huge difference. Thank you!

I was trying to verify wikipedia issue but I'm running into following bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1761754
(I see it even without your changes. Any idea about this one?)

See Also: → 1761754
Blocks: wr-todos
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: