Closed Bug 884376 Opened 7 years ago Closed 6 years ago

Skia assumes sparc is a little endia cpu

Categories

(Core :: Graphics, defect)

21 Branch
Sun
NetBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: martin, Assigned: stevensn)

References

Details

Attachments

(2 files, 2 obsolete files)

.. but certainly sparc is big endian
Furthermore SKIA_GPU has to be disabled for sparc as well, something like this to configure.in should do it:

@@ -8352,7 +8369,7 @@
     AC_DEFINE(GR_DLL)
   fi
 
-  if test "${CPU_ARCH}" != "ppc" -a "${CPU_ARCH}" != "ppc64"; then
+  if test "${CPU_ARCH}" != "ppc" -a "${CPU_ARCH}" != "ppc64" -a "${CPU_ARCH}" != "sparc"; then
     MOZ_ENABLE_SKIA_GPU=1
     AC_DEFINE(USE_SKIA_GPU)
     AC_SUBST(MOZ_ENABLE_SKIA_GPU)
I'd rather have it as a whitelist of archs supporting skia/gpu (ie tested on i386/amd64/arm ?) or based on endianness or... rather than having to blacklist every BE arch.

See also bug 849253. As for the patch against skia itself, it should either complete gfx/skia/patches/0018-Bug-817356-PPC-defines.patch or be made on top of it, so that we keep track of changes against upstream..
Also reported upstream: http://code.google.com/p/skia/issues/detail?id=1355
Duplicate of this bug: 948404
This patch updates the patch in gfx/skia/patches
It also adds other platforms to the list in SkPreConfig.h based on what we do in mfbt/Endian.h

I haven't update configure.in to use a whitelist for SKIA_GPU I can do that if we want
Attachment #764213 - Attachment is obsolete: true
Attachment #8351431 - Flags: review?(gwright)
Comment on attachment 8351431 [details] [diff] [review]
add more platforms to SKIA Big Endian list

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

I'm happy for the change to configure.in to land immediately, but please upstream the remaining hunks first as we now have an upstream-first policy for patches.
Attachment #8351431 - Flags: review?(gwright) → review+
Patch submitted upstream https://codereview.appspot.com/6900063/
Split out the configure.in portion of the patch
Attachment #8351431 - Attachment is obsolete: true
Attachment #8387343 - Flags: review+
the patches to the upstream components
checkin requested for Attachment #8387343 [details] [diff]
Keywords: checkin-needed
(In reply to Steve Singer (:stevensn) from comment #11)
> checkin requested for Attachment #8387343 [details] [diff] [diff]

a bit late to the party, but could we add sparc64 in there too ?
I'm pretty sure in this context ${CPU_ARCH} is "sparc" for sparc64, at least on NetBSD.
If the patch has been upstreamed there's no need to keep it around in gfx/skia/patches. I will be pruning that directory soon anyway.
(In reply to Martin Husemann from comment #13)
> I'm pretty sure in this context ${CPU_ARCH} is "sparc" for sparc64, at least
> on NetBSD.

you're right, it's set a little bit earlier in configure by

1171 sun4u | sparc*)
1172     CPU_ARCH=sparc
1173     ;;
https://hg.mozilla.org/mozilla-central/rev/81c5d568d949
Assignee: nobody → steve
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
(In reply to George Wright (:gw280) from comment #14)
> If the patch has been upstreamed there's no need to keep it around in
> gfx/skia/patches. I will be pruning that directory soon anyway.

I've submitted the patch upstream but it hasn't been applied yet.

Any hints on getting the patch moved through the SKIA reveiew/commit process?
(In reply to Steve Singer (:stevensn) from comment #18)
> Any hints on getting the patch moved through the SKIA reveiew/commit process?

Flag reed@google.com as the reviewer and ensure the "send mail to reviewers" option is checked.
You need to log in before you can comment on or make changes to this bug.