Closed Bug 1319374 Opened 3 years ago Closed 3 years ago

[--disable-skia] fatal error: skia/include/core/SkCanvas.h: No such file or directory

Categories

(Core :: Graphics, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 --- unaffected
firefox51 --- wontfix
firefox52 --- fixed
firefox53 --- fixed

People

(Reporter: glandium, Assigned: mchang)

References

Details

Attachments

(2 files)

Building with --disable-skia (which is the default when you build on a big-endian architecture (skia doesn't support big-endian)) fails with:

 0:00.64 In file included from gfx/layers/composite/LayerManagerComposite.cpp:14:0,
 0:00.64                  from obj-x86_64-pc-linux-gnu/gfx/layers/Unified_cpp_gfx_layers5.cpp:65:
 0:00.64 gfx/layers/composite/PaintCounter.h:12:40: fatal error: skia/include/core/SkCanvas.h: No such file or directory
This build break relatives to the feature of Bug 1294121. ni? Mason to figure out the better solution.
Flags: needinfo?(mchang)
Flags: needinfo?(mchang)
Probably just have to add MOZ_ENABLE_SKIA around some things. Do we even support big endian architectures?
Assignee: nobody → mchang
Should be USE_SKIA
It should build w/o disable-skia now, but it won't work on mac. 

Do we have any options on when we're going to stop enabling gecko builds without skia?
Attachment #8815112 - Flags: review?(lsalzman)
Attachment #8815112 - Flags: review?(lsalzman) → review+
https://hg.mozilla.org/mozilla-central/rev/c1e18cb7926e
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Too late for 51. Mark 51 won't fix.
Hi :mchang,
Do you think this is worth uplifting to Aurora52?
Flags: needinfo?(mchang)
(In reply to Gerry Chang [:gchang] from comment #9)
> Hi :mchang,
> Do you think this is worth uplifting to Aurora52?

Not really. We don't officially support any builds without skia. Some platforms won't even work without skia such as OS X.
Flags: needinfo?(mchang)
(In reply to Mason Chang [:mchang] from comment #10)
> (In reply to Gerry Chang [:gchang] from comment #9)
> > Hi :mchang,
> > Do you think this is worth uplifting to Aurora52?
> 
> Not really. We don't officially support any builds without skia. Some
> platforms won't even work without skia such as OS X.

The cutoff for officially retiring non-Skia builds is currently 53 (see bug 1323303). I would at least recommend keeping 52 (for ESR) in whatever quasi-unknown-working state we can. So as annoying as it is, if this breaks the non-Skia builds in 52, I would recommend uplifting this already. That way we can always point people to the ESR if they complain about Skia requirements in 53 onward.
The original patch bitrotted a bit against 52. I merely cleaned this up to apply to 52 but made no other changes.
Attachment #8828461 - Flags: review?(mchang)
Attachment #8828461 - Flags: review?(mchang) → review+
Comment on attachment 8828461 [details] [diff] [review]
Wrap PaintCounter with ifdef USE_SKIA (52)

Approval Request Comment
[Feature/Bug causing the regression]: bug 1294121
[User impact if declined]: Builds with Skia disabled will fail. This is mainly used for tier-3 platforms which we'd like to keep supporting up to the 52 ESR cutoff. With 53 onward we're no longer supporting building without Skia, but part of that plan is to point such users back to 52.
[Is this code covered by automated tests?]: no
[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]: aurora (52)
[Is the change risky?]: no
[Why is the change risky/not risky?]: Doesn't affect any of our normal builds which always use Skia, and we've already tested this in 52.
[String changes made/needed]: None
Attachment #8828461 - Flags: approval-mozilla-aurora?
Comment on attachment 8828461 [details] [diff] [review]
Wrap PaintCounter with ifdef USE_SKIA (52)

fix builds without skia, aurora52+
Attachment #8828461 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.