Closed Bug 849253 Opened 7 years ago Closed 7 years ago

skia doesnt build on big endian arch

Categories

(Core :: Graphics, defect)

PowerPC
OpenBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla22

People

(Reporter: gaston, Assigned: gw280)

References

Details

Attachments

(3 files, 3 obsolete files)

eg++ -o Factory.o -c  -fvisibility=hidden -DMOZ_GFX -DUSE_CAIRO -DGFX2D_INTERNAL -DMOZ_ENABLE_FREETYPE -DSK_A32_SHIFT=24 -DSK_R32_SHIFT=16 -DSK_G32_SHIFT=8 -DSK_B32_SHIFT
=0 -DMOZ_GLUE_IN_PROGRAM -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEB
ES -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT -DEXCLUDE_SKIA_DEPENDENCIES -DOS_POSIX=1 -DOS_OPENBSD=1 -DOS_BSD=1 -I/home/landry/src/mozilla-central/ipc/chromium/src 
-I/home/landry/src/mozilla-central/ipc/glue -I../../ipc/ipdl/_ipdlheaders  -I/home/landry/src/mozilla-central/gfx/2d -I. -I../../dist/include  -I/usr/obj/m-c/dist/include
/nspr -I/usr/obj/m-c/dist/include/nss      -fPIC -I/usr/X11R6/include -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-inval
id-offsetof -Wcast-align -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -pthread -pipe  -DNDEBUG -DTRIMME
D -g -fno-omit-frame-pointer  -I/usr/obj/m-c/dist/include/cairo   -I/usr/X11R6/include -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MF .deps/Factory.o.pp  /home/
landry/src/mozilla-central/gfx/2d/Factory.cpp
In file included from ../../dist/include/skia/GrSurface.h:12:0,
                 from ../../dist/include/skia/GrTexture.h:12,
                 from ../../dist/include/skia/GrPaint.h:13,
                 from ../../dist/include/skia/GrContext.h:14,
                 from /home/landry/src/mozilla-central/gfx/2d/DrawTargetSkia.h:8,
                 from /home/landry/src/mozilla-central/gfx/2d/Factory.cpp:14:
../../dist/include/skia/GrTypes.h:303:6: error: #error "Skia gpu currently assumes little endian"


Since a recent commit to skia it doesnt build anymore on powerpc. I've done a success build of m-c around the 24 of february.
Looking at what got recently commited, looks like a fallout of 751418. Should USE_SKIA be false on BE archs ?
Depends on: 751418
Now i'm pretty sure it's https://hg.mozilla.org/integration/mozilla-inbound/rev/4b6914fccc4f that is causing this - previously the gpu support in skia wasnt enabled so code under src/gpu wasnt reached, and now it is.

Now, what would be the best way to fix it ?
Don't build $(GPU_CPPSRCS) and $(GPU_GL_CPPSRCS) on big endian, and only define SK_SUPPORT_GPU on little endian platforms, I suspect.
(In reply to George Wright (:gw280) from comment #3)
> Don't build $(GPU_CPPSRCS) and $(GPU_GL_CPPSRCS) on big endian, and only
> define SK_SUPPORT_GPU on little endian platforms, I suspect.

Tried only #define SK_SUPPORT_GPU 0 first & then commenting out GPU_CPPSRCS/GPU_GL_CPPSRCS (with the #define still off), both failed at the same spot.  Changing that doesnt modify the way 2d/Factory.cpp include GrContext.h via DrawTargetSkia.h.....
Looking at http://code.google.com/p/skia/issues/detail?id=524 it seems upstream skia doesnt really care at all about big endian. So a proper way to disable skia/gpu on BE would be the way to go.
Tried disabling skia/gpu in the build itself wont fly, it seems like way too much #ifdef machinery. I see two options :
- Find the correct kSkia8888_GrPixelConfig value for BE archs and remove the #error ?
- remove the #error and return false in gfx/thebes/gfxPlatform.cpp:UseAcceleratedSkiaCanvas() ?
I removed the #ifndef  SK_CPU_LENDIAN check and got a firefox build.
Nothing obvious is broken after visiting a few sites.  Is there anything special (paramaters, or web sites that do fancy canvas stuff) I need to do to get skia to use the GPU?

Some of the SKIA code is BE aware, ie SkConfig8888.cpp has support for BE packings.
gfx.canvas.azure.backends = skia
gfx.canvas.azure.accelerated = true

That will enable the OpenGL backend for Skia. I suspect it won't work (or at least, will have R/B swapped).
(In reply to steve from comment #7)
> I removed the #ifndef  SK_CPU_LENDIAN check and got a firefox build.
> Nothing obvious is broken after visiting a few sites.  Is there anything
> special (paramaters, or web sites that do fancy canvas stuff) I need to do
> to get skia to use the GPU?

Care to share a diff ? I tried this, and it failed to build - but that might have been more unrelated fiddling i did locally.

(In reply to George Wright (:gw280) from comment #8)
> gfx.canvas.azure.backends = skia
> gfx.canvas.azure.accelerated = true
> 
> That will enable the OpenGL backend for Skia. I suspect it won't work (or at
> least, will have R/B swapped).

If it doesnt work, maybe it should return false in gfx/thebes/gfxPlatform.cpp:UseAcceleratedSkiaCanvas() on BE archs to be on the safe side.
Attached patch Build SkiaGL independently (obsolete) — Splinter Review
Here's an untested patch that makes SkiaGL independently buildable from Skia itself, and disabled by default on PPC. Please test as I don't have a PPC machine handy.
Attached is a patch that gets firefox compiling on ppc with SKIA.  Firefox works fine when SKIA is disabled but forcing azure to SKIA gives me segfaults in  mozilla::dom::CanvasRenderingContext2D::EnsureTarget  because GLContextProviderGLX::CreateOffscreen is returning nullptr.  I do not know if that is because of BE or unrelated issues with my graphics driver
I should also add that I had my patch from 846986 applied so I could link. 

I will test patch 724672 but the build will take a while
My patch is currently a little broken. It doesn't define MOZ_ENABLE_SKIA_GPU properly. I'll try and fix that later today.

I don't think your patch is going to work, unfortunately.
Attached patch Updated patch (obsolete) — Splinter Review
OK, this one should work...
Attachment #724672 - Attachment is obsolete: true
The updated patch doesn't build.

ake[6]: Entering directory `/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/build-output/gfx/2d'
c++ -o Factory.o -c  -fvisibility=hidden -DMOZ_GFX -DUSE_CAIRO -DGFX2D_INTERNAL -DMOZ_ENABLE_FREETYPE -DSK_A32_SHIFT=24 -DSK_R32_SHIFT=16 -DSK_G32_SHIFT=8 -DSK_B32_SHIFT=0 -DMOZ_GLUE_IN_PROGRAM -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES -DNO_NSPR_10_SUPPORT -DEXCLUDE_SKIA_DEPENDENCIES -DOS_POSIX=1 -DOS_LINUX=1 -I/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/ipc/chromium/src -I/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/ipc/glue -I../../ipc/ipdl/_ipdlheaders  -I/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/gfx/2d -I. -I../../dist/include  -I/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/build-output/dist/include/nspr -I/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/build-output/dist/include/nss      -fPIC  -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wno-invalid-offsetof -Wcast-align -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -g -Os -freorder-blocks  -fomit-frame-pointer -I/media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/build-output/dist/include/cairo    -DMOZILLA_CLIENT -include ../../mozilla-config.h -MD -MF .deps/Factory.o.pp  /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/gfx/2d/Factory.cpp
In file included from ../../dist/include/skia/GrSurface.h:12,
                 from ../../dist/include/skia/GrTexture.h:12,
                 from ../../dist/include/skia/GrPaint.h:13,
                 from ../../dist/include/skia/GrContext.h:14,
                 from /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/gfx/2d/DrawTargetSkia.h:8,
                 from /media/nfs/usb_drive_src/firefox/mozilla-central-hg/src/gfx/2d/Factory.cpp:14:
../../dist/include/skia/GrTypes.h:303:6: error: #error "Skia gpu currently assumes little endian"


grep SKIA config.status
    (''' MOZ_ENABLE_SKIA ''', ' 1 '),
    (''' USE_SKIA ''', ' 1 '),
    (''' MOZ_ENABLE_SKIA_GPU ''', r'''  '''),
    (''' MOZ_ENABLE_SKIA ''', r''' 1 '''),
    (''' MOZ_SKIA_LIBS ''', r''' $(DEPTH)/gfx/skia/$(LIB_PREFIX)skia.$(LIB_SUFFIX) '''),
Was your patch intended to be applied on-top of the one I posted?
(In reply to steve from comment #16)
> Was your patch intended to be applied on-top of the one I posted?

Nope. What architecture are you on?
Oh, I see what's wrong. Can you change:

#include "skia/GrContext.h"
#include "skia/SkCanvas.h"
#include "skia/GrContext.h"
#include "skia/GrGLInterface.h"

in DrawTargetSkia.h to:

#ifdef USE_SKIA_GPU
#include "skia/GrContext.h"
#include "skia/GrContext.h"
#include "skia/GrGLInterface.h"
#endif
#include "skia/SkCanvas.h"
Attached patch Updated patch v3 (obsolete) — Splinter Review
In addition to DrawTargetSkia.h I had to do something similar in HelpersSkia.h

I also had to make sure gets defined SK_SUPPORT_GPU 0 otherwise it was pulling defaulting to 1 in SkPostConfig.h and was pulling in GrTypes.h

Attached is an updated version of your patch with this changes.  It seems to compile but I haven't really tested it yet
I tried testing it but bug #840282 broke powerpc. (again.sigh.)
Firefox seems to be working fine with the patch. I haven't updated my repository since startinng on this so I am not yet hit by 840282
Attached patch patch v4Splinter Review
Final patch (hopefully) that's currently running through try.
Attachment #724789 - Attachment is obsolete: true
Attachment #725431 - Attachment is obsolete: true
Attachment #726436 - Flags: review?(matt.woodrow)
Firefox builds fine but crashes at startup for me with att 725431, but that might be related to #840282 - if it works for steve, that should be good for me too.
Comment on attachment 726436 [details] [diff] [review]
patch v4

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

::: configure.in
@@ +8286,5 @@
>    fi
> +
> +  if test "${CPU_ARCH}" != "ppc" -a "${CPU_ARCH}" != "ppc64"; then
> +    MOZ_ENABLE_SKIA_GPU=1
> +    AC_DEFINE(USE_SKIA_GPU)

Why MOZ_ for the makefile define and not for the c++ define?
Attachment #726436 - Flags: review?(matt.woodrow) → review+
(In reply to Matt Woodrow (:mattwoodrow) from comment #25)
> Comment on attachment 726436 [details] [diff] [review]
> patch v4
> 
> Review of attachment 726436 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: configure.in
> @@ +8286,5 @@
> >    fi
> > +
> > +  if test "${CPU_ARCH}" != "ppc" -a "${CPU_ARCH}" != "ppc64"; then
> > +    MOZ_ENABLE_SKIA_GPU=1
> > +    AC_DEFINE(USE_SKIA_GPU)
> 
> Why MOZ_ for the makefile define and not for the c++ define?

Mainly because that's what we currently do for the generic Skia defines. I filed bug 851212 related to this.
Blocks: 687187
I am still getting build failures with this.

central/gfx/2d/Scale.cpp
In file included from /media/nfs/usb_drive_src/firefox/mozilla-central/gfx/2d/HelpersSkia.h:13:0,
                 from /media/nfs/usb_drive_src/firefox/mozilla-central/gfx/2d/Scale.cpp:8:
../../dist/include/skia/GrTypes.h:303:6: error: #error "Skia gpu currently assumes little endian"

My attachement 725431 had changes to HelpersSkia.h that aren't in the patch that got committed
https://hg.mozilla.org/mozilla-central/rev/ee2bfeaaecaa
Assignee: nobody → gwright
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
I concur - you need the following missing chunk to build on BE.
Attachment #729606 - Flags: review?(gwright)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #729606 - Flags: review?(gwright) → review+
https://hg.mozilla.org/mozilla-central/rev/e2fb60b72822
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.