If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

gfx/layers/basic/BasicCompositor.cpp:20:56: fatal error: skia/SkCanvas.h: No such file or directory when skia is disabled

RESOLVED FIXED in Firefox 38

Status

()

Core
Graphics: Layers
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

(Depends on: 1 bug)

unspecified
mozilla39
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox36 wontfix, firefox37 affected, firefox38 fixed, firefox39 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
When building on non-i386, non-x86-64, non-arm, MOZ_ENABLE_SKIA defaults to "". This is equivalent to --disable-skia.

This yields the error in the summary.
(Assignee)

Updated

3 years ago
Depends on: 1137044
(Assignee)

Comment 1

3 years ago
Created attachment 8569614 [details] [diff] [review]
Remove duplicate SkDiscardableMemory_none.cpp from gfx/skia/moz.build

And to do so, cleanup gfx/skia/generate_mozbuild.py a little.

See bug #1137044 for what it takes to use generate_mozbuild.py to generate the same moz.build as what the patch yields.
Assignee: nobody → mh+mozilla
Attachment #8569614 - Flags: review?(gps)
(Assignee)

Updated

3 years ago
Blocks: 1097776
(Assignee)

Comment 2

3 years ago
Created attachment 8569616 [details] [diff] [review]
Always enable skia

The code that unconditionally includes skia headers has been added in bug 1097776, and aiui, removed the alternative, making akia mandatory.

So here is one way to deal with the problem: remove the option to do disable skia, considering building skia seems to work on the platforms it is currently disabled on (modulo the duplicate SkDiscardableMemory_none.cpp in gfx/skia/moz.build that is addressed in the other patch).

The alternative would be to half-backout bug 1097776, reintroducing the pixman code path when skia is not enabled.

I don't know which one is better, but at least the former is less involved for me.
Attachment #8569616 - Flags: review?(gwright)
(In reply to Mike Hommey [:glandium] from comment #2)
> So here is one way to deal with the problem: remove the option to do disable
> skia, considering building skia seems to work on the platforms it is
> currently disabled on (modulo the duplicate SkDiscardableMemory_none.cpp in
> gfx/skia/moz.build that is addressed in the other patch).
> 
> The alternative would be to half-backout bug 1097776, reintroducing the
> pixman code path when skia is not enabled.
> 
> I don't know which one is better, but at least the former is less involved
> for me.

I think we need to talk to the gfx team as to whether we are interested in supporting non-skia enabled builds anymore. If not, then let's remove the --disable-skia option entirely. If we do, then let's half-backout bug 1097776.
Flags: needinfo?(milan)

Comment 4

3 years ago
Comment on attachment 8569614 [details] [diff] [review]
Remove duplicate SkDiscardableMemory_none.cpp from gfx/skia/moz.build

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

::: gfx/skia/generate_mozbuild.py
@@ +197,1 @@
>      f.close()

might as well use the context manager version of open while you are here.
Attachment #8569614 - Flags: review?(gps) → review+
Comment on attachment 8569616 [details] [diff] [review]
Always enable skia

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

I'm ok with this if we think dropping --disable-skia is ok (I have no opposition to it personally).
Attachment #8569616 - Flags: review?(gwright) → review+
So I'm not sure I understand here. I get the error as in the summary for some platforms on version 36 but the patch(es) neither do apply to 36 nor do they work there.
At this moment I'm more interested to fix version 36. Any hints?
(Assignee)

Comment 7

3 years ago
(In reply to Wolfgang Rosenauer [:wolfiR] from comment #6)
> So I'm not sure I understand here. I get the error as in the summary for
> some platforms on version 36 but the patch(es) neither do apply to 36 nor do
> they work there.

The patch works... on aarch64 (but then the build fails somewhere else)

> At this moment I'm more interested to fix version 36. Any hints?

See comment 2.
(Assignee)

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8a4477dd1516
(Assignee)

Updated

3 years ago
Keywords: leave-open
(Assignee)

Updated

3 years ago
Attachment #8569614 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/8a4477dd1516
(Assignee)

Comment 10

3 years ago
Created attachment 8574516 [details] [diff] [review]
Reintroduce pixman code path removed in bug 1097776 for --disable-skia builds

Skia simply doesn't build on non-arm, non-x86, so until it does, it's simpler to just half-backout bug 1097776.
Attachment #8569616 - Attachment is obsolete: true
Attachment #8574516 - Flags: review?(jmuizelaar)
Whiteboard: gfx-noted
Confirming that skia doesnt build on sparc64... testing att 8574516
Related: bug 1005535 tries to enable gpu for skia on ppc, hopefully fixing big endian platforms..
Attachment #8574516 - Flags: review?(jmuizelaar) → review+
With attachment 8574516 [details] [diff] [review], i still get a build failure on OpenBSD/sparc64.

error: #error "SK_*32_SHFIT values must correspond to BGRA or RGBA byte order"

Cf tail of http://buildbot.rhaalovely.net/builders/mozilla-central-sparc64/builds/1001/steps/build/logs/stdio
Sounds like --disable-skia wasn't honoured?
(Assignee)

Comment 15

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/49ccfbcb685a
(Assignee)

Updated

3 years ago
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/49ccfbcb685a
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox39: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

Comment 17

3 years ago
Created attachment 8578600 [details] [diff] [review]
skia big/little endian patch

(In reply to Landry Breuil (:gaston) from comment #13)
> With attachment 8574516 [details] [diff] [review], i still get a build
> failure on OpenBSD/sparc64.
> 
> error: #error "SK_*32_SHFIT values must correspond to BGRA or RGBA byte
> order"

Try this patch I use for Fedora.
Mike, is this something that could be uplifted to 38 so that it makes the ESR?
Flags: needinfo?(mh+mozilla)
(Assignee)

Updated

3 years ago
Flags: needinfo?(mh+mozilla)
(Assignee)

Updated

3 years ago
Flags: needinfo?(milan)
(Assignee)

Comment 19

3 years ago
Comment on attachment 8574516 [details] [diff] [review]
Reintroduce pixman code path removed in bug 1097776 for --disable-skia builds

Approval Request Comment
[Feature/regressing bug #]: bug 1097776
[User impact if declined]: Can't build Firefox without skia, while skia doesn't support all esoteric platforms.
[Describe test coverage new/current, TreeHerder]: Landed on m-c yesterday, used on Debian for a few weeks.
[Risks and why]: In practice, this patch doesn't change anything for Mozilla builds, and restores the non-skia code path that was removed in bug 1097776.
[String/UUID change made/needed]: None
Attachment #8574516 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 1105087
status-firefox36: affected → wontfix
Attachment #8574516 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/48e130d69836
status-firefox38: affected → fixed
You need to log in before you can comment on or make changes to this bug.