Closed Bug 1447913 Opened 2 years ago Closed 2 years ago

gfx/skia/skia/include/core/SkImage.h:11:10: fatal error: 'GrTypes.h' file not found (--disable-skia-gpu, default on OpenBSD/NetBSD)

Categories

(Core :: Graphics: Text, defect)

Unspecified
OpenBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla61
Tracking Status
firefox-esr52 --- unaffected
firefox59 --- unaffected
firefox60 --- fixed
firefox61 --- fixed

People

(Reporter: gaston, Assigned: lsalzman)

References

Details

(Keywords: regression)

Attachments

(1 file, 3 obsolete files)

m-c fails to build on openbsd in skia, probably because of #1234494 which disabled skia gpu there (i guess because GrTypes.h is in include/gpu and its not in the search path or something ?):

 0:53.57 /usr/bin/clang++ -std=gnu++14 -o Unified_cpp_gfx_ycbcr0.o -c -I/usr/obj/m-c/dist/stl_wrappers -I/usr/obj/m-c/dist/system_wrappers -include /home/landry/src/m-c/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/home/landry/src/m-c/gfx/ycbcr -I/usr/obj/m-c/gfx/ycbcr -I/home/landry/src/m-c/media/libyuv/libyuv/include -I/usr/obj/m-c/dist/include -I/usr/obj/m-c/dist/include/nspr -I/usr/obj/m-c/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /usr/obj/m-c/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I/usr/X11R6/include -Qunused-arguments -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wloop-analysis -Wc++1z-compat -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -pthread -pipe -O -fno-omit-frame-pointer  -MD -MP -MF .deps/Unified_cpp_gfx_ycbcr0.o.pp  -fcolor-diagnostics  /usr/obj/m-c/gfx/ycbcr/Unified_cpp_gfx_ycbcr0.cpp                                                                                   
 0:54.13 In file included from /home/landry/src/m-c/gfx/2d/DrawTargetSkia.cpp:7:                                                           
 0:54.13 In file included from /home/landry/src/m-c/gfx/2d/DrawTargetSkia.h:11:                                                            
 0:54.13 In file included from /home/landry/src/m-c/gfx/skia/skia/include/core/SkSurface.h:12:                                             
 0:54.13 /home/landry/src/m-c/gfx/skia/skia/include/core/SkImage.h:11:10: fatal error: 'GrTypes.h' file not found                          
 0:54.13 #include "GrTypes.h"

I'll see if skia gpu bits now build on openbsd, but maybe some missing #ifdefs..

Bug is similar to #1299485.
Blocks: 1444506
Keywords: regression
OS: Unspecified → OpenBSD
Summary: non-skia gpu (openbsd/netbsd) builds broken, GrTypes.h not found → gfx/skia/skia/include/core/SkImage.h:11:10: fatal error: 'GrTypes.h' file not found (--disable-skia-gpu, default on OpenBSD/NetBSD)
Thx jan for linking to the right bug - i see that previously in gfx/2d/HelpersSkia.h the inclusion of GrTypes.h was within 

#ifdef USE_SKIA_GPU
#include "skia/include/gpu/GrTypes.h"
#endif

so it's probably the way to go, but better if we can avoid deviating from other platforms and build skia gpu.
Fwiw we should probably be able to just enable skia gpu on OpenBSD, since it seems we have a 'better' locale support with https://github.com/openbsd/src/commit/3a628b46e7aaa520a6215eccabf31d313c2e7de0

At least locale_t is defined, so the build failures fixed in #1234494 shouldnt happen anymore - build is still running here, not yet reached libxul linking, but if it succeeds maybe we're fine with skia gpu enabled.
It is unlikely that we ever will implement the per thread locale support, our locale team strongly objects the idea.
Attached patch enable skia gpu on OpenBSD (obsolete) — Splinter Review
I dunno how to handle this for NetBSD, but for OpenBSD and our current locale_t support skia builds fine with gpu support, and the resulting binary runs - dunno how to specifically exercise skia gpu features though.

Of course will also need backporting to beta/60..
Assignee: nobody → landry
Attachment #8961326 - Flags: review?(lsalzman)
Comment on attachment 8961327 [details]
Bug 1447913 - Enable Skia GPU on OpenBSD.

Landry, can you try this instead? Dropping/adjusting --disable-skia-gpu maybe too late for Firefox 60.
Attachment #8961327 - Flags: feedback?(landry)
Maybe related (or not) but the given binary consumes a hell of a lot cputime. Will have to look at what can be disabled at runtime..
Comment on attachment 8961327 [details]
Bug 1447913 - Enable Skia GPU on OpenBSD.

This version also builds for me, but i'd rather having us coming back to the common case inline with other platforms.. seems it consumes less cpu cycles for nothing though (compared to my build with skia gpu enabled)
Attachment #8961327 - Flags: feedback?(landry) → feedback+
Comment on attachment 8961326 [details] [diff] [review]
enable skia gpu on OpenBSD

Landry, did you check Skia GPU runtime on OpenBSD i386? It maybe risky to backport to ESR60 otherwise.
(In reply to Jan Beich from comment #10)
> Comment on attachment 8961326 [details] [diff] [review]
> enable skia gpu on OpenBSD
> 
> Landry, did you check Skia GPU runtime on OpenBSD i386? It maybe risky to
> backport to ESR60 otherwise.

Native i386 is dead, rust killed it (3gb memspace). And i doubt it'll come back... but who knows :)
See Also: → 1447962
Comment on attachment 8961326 [details] [diff] [review]
enable skia gpu on OpenBSD

Moved to attachment 8961327 [details] in order to fix commit message, NetBSD comment and push to Try as a patchset.
Attachment #8961326 - Attachment is obsolete: true
Attachment #8961326 - Flags: review?(lsalzman)
So as not to drive me crazy with a bunch of different patches from different people on different bugs, I am combining all these into an omnibus patch that should do the same thing.

Enables Skia GPU support by default on all platforms now, but fixes the include handling issue in case someone does still want to use --disable-skia-gpu explicitly in their builds.
Assignee: landry → lsalzman
Attachment #8961327 - Attachment is obsolete: true
Attachment #8961469 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8961327 - Flags: review?(lsalzman)
Attachment #8961469 - Flags: review?(lsalzman)
Attachment #8961470 - Flags: review?(jbeich)
Comment on attachment 8961470 [details] [diff] [review]
fix building Skia on BSDs

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

::: gfx/skia/skia/include/config/SkUserConfig.h
@@ +167,5 @@
> +#ifndef SK_SUPPORT_GPU
> +#  ifdef USE_SKIA_GPU
> +#    define SK_SUPPORT_GPU 1
> +#  else
> +#    define SK_SUPPORT_GPU 0

Looks better than my version. Thank you.
Attachment #8961470 - Flags: review?(jbeich) → review+
See Also: 1447962
Duplicate of this bug: 1447962
Blows on taskcluster, cf https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=bdf1b3096b346d457ddea59c1ff7ff2babaefc99&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable&filter-resultStatus=success&filter-resultStatus=running&filter-resultStatus=pending

extract from the log:

[task 2018-03-22T18:48:47.477Z] 18:48:47     INFO - E                   ConfigureError: /builds/worker/workspace/build/src/toolkit/moz.configure:899: The dependency on `target` is unused.
[task 2018-03-22T18:48:47.477Z] 18:48:47     INFO - ../python/mozbuild/mozbuild/configure/lint.py:74: ConfigureError

skia_gpu method has a 'target' argument which is now unused.
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/93b50e176faa
follow-up - fix skia_gpu dependency on target. r=me CLOSED TREE
Can you request backport to Beta aka Firefox 60 (ESR) ?
Flags: needinfo?(lsalzman)
https://hg.mozilla.org/mozilla-central/rev/bdf1b3096b34
https://hg.mozilla.org/mozilla-central/rev/93b50e176faa
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment on attachment 8961470 [details] [diff] [review]
fix building Skia on BSDs

Approval Request Comment
[Feature/Bug causing the regression]: bug 1444506 
[User impact if declined]: Broken build on OpenBSD and NetBSD. Cannot be worked around via --disable-skia as it isn't supported after bug 1323303.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: mozilla-central changeset 93b50e176faa
[Is the change risky?]: No
[Why is the change risky/not risky?]: Can only break build.
[String changes made/needed]: None
Flags: needinfo?(lsalzman)
Attachment #8961470 - Flags: approval-mozilla-beta?
Comment on attachment 8961470 [details] [diff] [review]
fix building Skia on BSDs

skia build fix for bsd, beta60+
Attachment #8961470 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.