1323303 - Remove option to configure builds without Skia

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Milan Sreckovic [:milan] (needinfo for best results)]

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.)

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

Posted to dev-platform.

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: require building with Skia

As discussed at the All Hands, remove support for --disable-skia builds.

Comment 3

[Nicholas Nethercote [inactive]]

Are you going to remove MOZ_ENABLE_SKIA and USE_SKIA?

Comment 4

[Lee Salzman [:lsalzman]]

(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.

Comment 5

[George Wright (:gw280) (needinfo me!)]

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.

Comment 6

[Lee Salzman [:lsalzman]]

(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.

Comment 7

[George Wright (:gw280) (needinfo me!)]

(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

[Mike Hommey [:glandium]]

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

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/28c47fbedac4
require building with Skia. r=glandium

Comment 10

[Carsten Book [:Tomcat]]

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=40878666&repo=mozilla-inbound

Comment 11

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/73c80c1709a3
Backed out changeset 28c47fbedac4 for bustage

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d8e95d80c65f
require building with Skia. r=glandium

Comment 13

[Lee Salzman [:lsalzman]]

(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!

Comment 14

[Adam Moore]

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

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d8e95d80c65f

Comment 16

[George Wright (:gw280) (needinfo me!)]

(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

[Adam Moore]

(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

[Ian Stakenvicius]

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)

Comment 19

[Lee Salzman [:lsalzman]]

(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

[Ian Stakenvicius]

(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.

Comment 21

[Lee Salzman [:lsalzman]]

(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.