Bug 146680
| Summary: | [GTK][WPE] Replace OpenGL{,ES}Shims with libepoxy, and #if USE(OPENGL_ES_2) with runtime checks | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Emmanuel Gil Peyrot <emmanuel.peyrot> |
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | Enhancement | CC: | aperez, bugs-noreply, cgarcia, clopez, contact+bugs.webkit.org, dvpdiner2, magomez, mcatanzaro, mrobinson, webkit-bug-importer, yoon, ysuzuki, zan |
| Priority: | P2 | Keywords: | InRadar |
| Version: | 528+ (Nightly build) | ||
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://github.com/anholt/libepoxy | ||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=172104 https://bugs.webkit.org/show_bug.cgi?id=236593 https://bugs.webkit.org/show_bug.cgi?id=248074 |
||
Emmanuel Gil Peyrot
libepoxy is a library retrieving function pointers from a libGL, one of the many OpenGL wranglers you might have hard from.
What makes it better than the current solution is its automated aliasing of identical functions, and its ability to select a libGL or libGLESv2 at runtime instead of at compile-time, so no recompilation with -DENABLE_GLES2=ON and OFF required anymore to support both APIs on multiple drivers, everything in a single binary.
In most case, #if USE(OPENGL_ES_2) would be replaced with a cached if (!epoxy_is_desktop_gl()), cached to avoid a potential runtime string comparison everytime the function is called.
libepoxy is a dependency of GTK+ and Xorg (for GLAMOR), and probably some other widely used software, so this usually won’t add another dependency.
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Martin Robinson
Using libepoxy would be excellent, though we have a policy of not adding mandatory dependencies between major releases.
Carlos Alberto Lopez Perez
(In reply to comment #1)
> Using libepoxy would be excellent, though we have a policy of not adding
> mandatory dependencies between major releases.
What does exactly mean "between major releases" ?
Does it mean that is ok to merge a patch that adds a dependency on libepoxy on trunk, but is not ok to backport that patch to the stable series (2.8.x)?
Martin Robinson
(In reply to comment #2)
> (In reply to comment #1)
> > Using libepoxy would be excellent, though we have a policy of not adding
> > mandatory dependencies between major releases.
>
> What does exactly mean "between major releases" ?
>
> Does it mean that is ok to merge a patch that adds a dependency on libepoxy
> on trunk, but is not ok to backport that patch to the stable series (2.8.x)?
Essentially we will not bump required dependencies until the 3.x series.
Carlos Alberto Lopez Perez
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Using libepoxy would be excellent, though we have a policy of not adding
> > > mandatory dependencies between major releases.
> >
> > What does exactly mean "between major releases" ?
> >
> > Does it mean that is ok to merge a patch that adds a dependency on libepoxy
> > on trunk, but is not ok to backport that patch to the stable series (2.8.x)?
>
> Essentially we will not bump required dependencies until the 3.x series.
There was a discussion [1] on the mailing list regarding our policy for bumping dependencies that resulted in an agreement of defining a more pragmatic policy.
https://trac.webkit.org/wiki/WebKitGTK/DependenciesPolicy
According to it, its now ok to introduce the dependency on libepoxy.
[1] https://trac.webkit.org/wiki/WebKitGTK/DependenciesPolicy
Carlos Garcia Campos
libepoxy is a small library, we could also add it to ThirdParty if adding the dependency would be a problem
Martin Robinson
(In reply to comment #5)
> libepoxy is a small library, we could also add it to ThirdParty if adding
> the dependency would be a problem
I think that would be better than adding a new dependency, which I think is something that the policy didn't cover.
Carlos Garcia Campos
(In reply to comment #6)
> (In reply to comment #5)
> > libepoxy is a small library, we could also add it to ThirdParty if adding
> > the dependency would be a problem
>
> I think that would be better than adding a new dependency, which I think is
> something that the policy didn't cover.
Yes, also because the idea would be to use it in WebCore for more ports, not just GTK+, and other ports might have other policies.
Carlos Alberto Lopez Perez
(In reply to comment #5)
> libepoxy is a small library, we could also add it to ThirdParty if adding
> the dependency would be a problem
GTK+ already depended on libepoxy (since 3.18 i believe). So it will not be a real new dependency at least for WebKitGTK+. But I understand the concern for other ports. Would this patch implement something that will be used for WebKitGTK+ or it will be shared with other ports?
GTK+ depends on libepoxy.(In reply to comment #6)
> (In reply to comment #5)
> > libepoxy is a small library, we could also add it to ThirdParty if adding
> > the dependency would be a problem
>
> I think that would be better than adding a new dependency, which I think is
> something that the policy didn't cover.
Well, my reading of the policy is that is ok to add a new dependency meanwhile at least one of this two preconditions is true:
- The dependency is needed for building WebKitGTK+ and Debian/Ubuntu already ships that dependency. The policy is : "WebKitGTK+ should be build-adble on the last Ubuntu LTS and last Debian stable" + 1 year since a new Debian/UbuntuLTS is released.
- The dependency is optional (ex: it is needed for some feature that can be disabled)
But we should clarify this.. do you think is worth raising the topic another time on the mailing list?
Michael Catanzaro
The intent behind the policy (at least, my intent) is to not introduce dependencies that are too new for our target distros. Adding a new dependency that already exists in both target distros (Ubuntu 14.04, Debian Jessie), which will not cause any build problems, seems fine to me.
Ubuntu 14.04 has libepoxy 1.1, so we can add a dependency on that. If we want to add a dependency on libepoxy 1.2, then we'd need to wait until next April, when our target switches from Ubuntu 14.04 to Ubuntu 16.04.
(In reply to comment #8)
> GTK+ already depended on libepoxy (since 3.18 i believe). So it will not be
> a real new dependency at least for WebKitGTK+. But I understand the concern
> for other ports. Would this patch implement something that will be used for
> WebKitGTK+ or it will be shared with other ports?
And, in particular, what other ports? If it's just EFL, they target Ubuntu; I strongly suspect they would be happy to use system libepoxy. If it would be needed for any Apple ports, then it needs to be bundled. I'm not sure about WinCairo.
From a downstream perspective: if you bundle libepoxy, we will rm -rf it in our spec file and patch the build to use system libepoxy. I suspect Berto will want something similar. So it would be greatly preferred to not bundle it, or to use the bundled version only for other ports if required and not distribute it with WebKitGTK+.
Michael Catanzaro
Also I agree that, in general, we can add new dependencies if they're only required for optional features, like OpenGL. But I wouldn't use that in this case, since we surely don't want any downstreams disabling OpenGL support.
Carlos Garcia Campos
(In reply to comment #8)
> (In reply to comment #5)
> > libepoxy is a small library, we could also add it to ThirdParty if adding
> > the dependency would be a problem
>
> GTK+ already depended on libepoxy (since 3.18 i believe). So it will not be
> a real new dependency at least for WebKitGTK+. But I understand the concern
> for other ports. Would this patch implement something that will be used for
> WebKitGTK+ or it will be shared with other ports?
It's a real dependency as long as we still support older versions of GTK+. It will be shared.
> GTK+ depends on libepoxy.(In reply to comment #6)
> > (In reply to comment #5)
> > > libepoxy is a small library, we could also add it to ThirdParty if adding
> > > the dependency would be a problem
> >
> > I think that would be better than adding a new dependency, which I think is
> > something that the policy didn't cover.
>
> Well, my reading of the policy is that is ok to add a new dependency
> meanwhile at least one of this two preconditions is true:
>
> - The dependency is needed for building WebKitGTK+ and Debian/Ubuntu
> already ships that dependency. The policy is : "WebKitGTK+ should be
> build-adble on the last Ubuntu LTS and last Debian stable" + 1 year since a
> new Debian/UbuntuLTS is released.
> - The dependency is optional (ex: it is needed for some feature that can be
> disabled)
>
> But we should clarify this.. do you think is worth raising the topic another
> time on the mailing list?
Carlos Garcia Campos
(In reply to comment #9)
> The intent behind the policy (at least, my intent) is to not introduce
> dependencies that are too new for our target distros. Adding a new
> dependency that already exists in both target distros (Ubuntu 14.04, Debian
> Jessie), which will not cause any build problems, seems fine to me.
>
> Ubuntu 14.04 has libepoxy 1.1, so we can add a dependency on that. If we
> want to add a dependency on libepoxy 1.2, then we'd need to wait until next
> April, when our target switches from Ubuntu 14.04 to Ubuntu 16.04.
>
> (In reply to comment #8)
> > GTK+ already depended on libepoxy (since 3.18 i believe). So it will not be
> > a real new dependency at least for WebKitGTK+. But I understand the concern
> > for other ports. Would this patch implement something that will be used for
> > WebKitGTK+ or it will be shared with other ports?
>
> And, in particular, what other ports? If it's just EFL, they target Ubuntu;
> I strongly suspect they would be happy to use system libepoxy. If it would
> be needed for any Apple ports, then it needs to be bundled. I'm not sure
> about WinCairo.
GTK+, EFL and Windows. I'm not sure about mac.
> From a downstream perspective: if you bundle libepoxy, we will rm -rf it in
> our spec file and patch the build to use system libepoxy. I suspect Berto
> will want something similar. So it would be greatly preferred to not bundle
> it, or to use the bundled version only for other ports if required and not
> distribute it with WebKitGTK+.
I don't like adding projects to ThirdParty either, but sometimes there's no other way if we need to add a new dependency that is mandatory. Using libepoxy conditionally doesn't make sense, I think.
Emmanuel Gil Peyrot
> Using libepoxy conditionally doesn't make sense, I think.
This was indeed the intent of this ticket, making distribution easier by having a single binary targetting both desktop GL and GLES (especially useful on ARM devices), and making code maintenance easier by merging the two codepaths wherever it makes sense.
Michael Catanzaro
(In reply to comment #12)
> I don't like adding projects to ThirdParty either, but sometimes there's no
> other way if we need to add a new dependency that is mandatory. Using
> libepoxy conditionally doesn't make sense, I think.
I don't mean use it conditionally; that doesn't make sense. I mean use the version under ThirdParty for Windows (and maybe Mac?), and get it from the system on Linux (because almost all distros will patch to get the system version regardless, so it's nicer to handle it upstream).
Yusuke Suzuki
Now, April is over. So I think we can reconsider about doing this :)
Michael Catanzaro
Closing after bug #248074. GTK now uses libepoxy. The other suggested changes are no longer relevant.
Radar WebKit Bug Importer
<rdar://problem/102586769>