Remove option to configure builds without Skia

RESOLVED FIXED in Firefox 53

Status

()

Core
Graphics
P1
normal
RESOLVED FIXED
a year ago
8 days ago

People

(Reporter: milan, Assigned: lsalzman)

Tracking

unspecified
mozilla53
Unspecified
All
Points:
---

Firefox Tracking Flags

(firefox53 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(1 attachment)

(Reporter)

Description

a year ago
With Skia being the only supported software backend, we're going to remove the build option to skip it.  We will take patches to fix build and runtime issues with Skia (we don't anticipate SkiaGL patches showing up.)
(Reporter)

Comment 1

a year ago
Posted to dev-platform.
Assignee: nobody → lsalzman
Priority: -- → P1
Whiteboard: [gfx-noted]
(Reporter)

Updated

a year ago
See Also: → bug 1144632, bug 836494
(Assignee)

Comment 2

11 months ago
Created attachment 8819146 [details] [diff] [review]
require building with Skia

As discussed at the All Hands, remove support for --disable-skia builds.
Attachment #8819146 - Flags: review?(mh+mozilla)
Are you going to remove MOZ_ENABLE_SKIA and USE_SKIA?
(Assignee)

Comment 4

11 months ago
(In reply to Nicholas Nethercote [:njn] from comment #3)
> Are you going to remove MOZ_ENABLE_SKIA and USE_SKIA?

For now, this just removes the option to disable it at all in the config.

Removing the macros would break a lot of code. As a follow-up bug, we can investigate removing all the #ifdef-ery that depends on USE_SKIA and friends.
What would it break? I don't recall any places that would be problematic here.

In any case, I think we should remove MOZ_ENABLE_SKIA and USE_SKIA. Going forward we aren't going to be supporting these being defined to 0 so it'll likely just bitrot over time.
(Assignee)

Comment 6

11 months ago
(In reply to George Wright (:gw280) (:gwright) from comment #5)
> What would it break? I don't recall any places that would be problematic
> here.
> 
> In any case, I think we should remove MOZ_ENABLE_SKIA and USE_SKIA. Going
> forward we aren't going to be supporting these being defined to 0 so it'll
> likely just bitrot over time.

I mean that until such time as all the #ifdef USE_SKIA usage is fixed, and all the moz.builds are fixed, it isn't quite safe yet to just remove from the config. That can surely be done soon as well, but I just wanted to get the actual issue of forcing everyone to build with Skia out of the way ASAP.
(In reply to Lee Salzman [:lsalzman] from comment #6)
> (In reply to George Wright (:gw280) (:gwright) from comment #5)
> > What would it break? I don't recall any places that would be problematic
> > here.
> > 
> > In any case, I think we should remove MOZ_ENABLE_SKIA and USE_SKIA. Going
> > forward we aren't going to be supporting these being defined to 0 so it'll
> > likely just bitrot over time.
> 
> I mean that until such time as all the #ifdef USE_SKIA usage is fixed, and
> all the moz.builds are fixed, it isn't quite safe yet to just remove from
> the config. That can surely be done soon as well, but I just wanted to get
> the actual issue of forcing everyone to build with Skia out of the way ASAP.

Oh I misinterpreted what you wrote. I thought we were discussing removing them wholesale from the entire codebase, not just configure.

I think we're in agreement then!

Comment 8

11 months ago
Comment on attachment 8819146 [details] [diff] [review]
require building with Skia

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

::: toolkit/moz.configure
@@ +767,5 @@
>  
>  @depends('--disable-skia', target)
>  def skia(value, target):
> +    if not value:
> +        die('--disable-skia is not supported')

+ anymore
Attachment #8819146 - Flags: review?(mh+mozilla) → review+

Comment 9

11 months ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/28c47fbedac4
require building with Skia. r=glandium

Comment 10

11 months ago
backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=40878666&repo=mozilla-inbound
Flags: needinfo?(lsalzman)

Comment 11

11 months ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73c80c1709a3
Backed out changeset 28c47fbedac4 for bustage

Comment 12

11 months ago
Pushed by lsalzman@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e95d80c65f
require building with Skia. r=glandium
(Assignee)

Comment 13

11 months ago
(In reply to Carsten Book [:Tomcat] from comment #10)
> backed out for bustage like
> https://treeherder.mozilla.org/logviewer.html#?job_id=40878666&repo=mozilla-
> inbound

Foiled by lint... Should be fixed now!
Flags: needinfo?(lsalzman)

Comment 14

11 months ago
Do you intend to remove cairo support?  I'm fine with having to build with skia support, but no cairo support would cause me problems.

Comment 15

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d8e95d80c65f
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(In reply to Adam Moore from comment #14)
> Do you intend to remove cairo support?  I'm fine with having to build with
> skia support, but no cairo support would cause me problems.

Eventually, but no immediate plans afaik. What problems will it cause you?

Comment 17

11 months ago
(In reply to George Wright (:gw280) (:gwright) from comment #16)
> Eventually, but no immediate plans afaik. What problems will it cause you?

We have an unusual setup.  We are launching Firefox on a remote server and sending the display bits back to a client app.  Our display stack has optimizations built around the data we normally would get from the X11 render path, and we are not getting what we need if we use skia for content rendering.

Comment 18

8 months ago
Is there any solution for skia support on big-endian platforms now that this is official?  Or will Firefox-53 officially kill support for ppc,ppc64,sparc,etc ?

(see bug 1144632)
(Assignee)

Comment 19

8 months ago
(In reply to Ian Stakenvicius from comment #18)
> Is there any solution for skia support on big-endian platforms now that this
> is official?  Or will Firefox-53 officially kill support for
> ppc,ppc64,sparc,etc ?
> 
> (see bug 1144632)

Any big-endian platforms are tier-3. That is much different from killing support. We still take contributor patches to resolve issues, and we are still maintaining the possibility, if not the actuality, of running on big-endian platforms in the code. That is to say, we are not purposely trying to make the situation worse internally, but it may coincidentally degrade over time due to bitrot if there are not sufficient contributed patches to offset it.

But Skia in principle, SkiaGL excepted, should work on big-endian. We just don't have a way internally to build/test/verify the result.

Comment 20

8 months ago
(In reply to Lee Salzman [:lsalzman] from comment #19)
> (In reply to Ian Stakenvicius from comment #18)
> > Is there any solution for skia support on big-endian platforms now that this
> > is official?  Or will Firefox-53 officially kill support for
> > ppc,ppc64,sparc,etc ?
> > 
> > (see bug 1144632)
> 
> Any big-endian platforms are tier-3. That is much different from killing
> support. We still take contributor patches to resolve issues, and we are
> still maintaining the possibility, if not the actuality, of running on
> big-endian platforms in the code. That is to say, we are not purposely
> trying to make the situation worse internally, but it may coincidentally
> degrade over time due to bitrot if there are not sufficient contributed
> patches to offset it.
> 
> But Skia in principle, SkiaGL excepted, should work on big-endian. We just
> don't have a way internally to build/test/verify the result.

Oh absolutely, I understand -- I didn't mean to imply that there was any malicious intent or anything regarding these tier-3 platforms, I was just trying to get at whether or not it's going to be feasible or possible to still make firefox work on them after ESR52.  IE, if this would be the proverbial final straw for firefox on big-endian platforms.
(Assignee)

Comment 21

8 months ago
(In reply to Ian Stakenvicius from comment #20)
> (In reply to Lee Salzman [:lsalzman] from comment #19)
> > (In reply to Ian Stakenvicius from comment #18)
> > > Is there any solution for skia support on big-endian platforms now that this
> > > is official?  Or will Firefox-53 officially kill support for
> > > ppc,ppc64,sparc,etc ?
> > > 
> > > (see bug 1144632)
> > 
> > Any big-endian platforms are tier-3. That is much different from killing
> > support. We still take contributor patches to resolve issues, and we are
> > still maintaining the possibility, if not the actuality, of running on
> > big-endian platforms in the code. That is to say, we are not purposely
> > trying to make the situation worse internally, but it may coincidentally
> > degrade over time due to bitrot if there are not sufficient contributed
> > patches to offset it.
> > 
> > But Skia in principle, SkiaGL excepted, should work on big-endian. We just
> > don't have a way internally to build/test/verify the result.
> 
> Oh absolutely, I understand -- I didn't mean to imply that there was any
> malicious intent or anything regarding these tier-3 platforms, I was just
> trying to get at whether or not it's going to be feasible or possible to
> still make firefox work on them after ESR52.  IE, if this would be the
> proverbial final straw for firefox on big-endian platforms.

We are, as far as anyone can tell, keeping around big-endian support, though as tier 3. We have not many any plans to retire it.
You need to log in before you can comment on or make changes to this bug.