1318201 - Remove link to EGL on linux

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Sotaro Ikeda [:sotaro]]

On linux, we just use GLX. We do not need to link to EGL.

With https://github.com/jrmuizel/gecko-dev/pull/31, source of offscreen_gl_context is in third_party/rust/offscreen_gl_context/. By modifying the source, we could remove the dependency to EGL.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Agreed, I did a try push to verify that we can remove the EGL dependency. I'll submit a PR to the offscreen_gl_context repo tomorrow if you don't get to it first.

I used the patch at [1] to verify, full try push is at [2].

[1] https://hg.mozilla.org/try/rev/25fb3bea2001897e8400c502eb00e169dfdcd1d8
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1925c585b7f38a7cdc0e06391c4fa826f55fc943

Comment 2

[Sotaro Ikeda [:sotaro]]

Assign to :kats since he already has patch to do it. I was  tying to find a way to update .cargo-checksum.json. :kats how do you update the .cargo-checksum.json?

Comment 3

[Sotaro Ikeda [:sotaro]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #1)
> Agreed, I did a try push to verify that we can remove the EGL dependency.
> I'll submit a PR to the offscreen_gl_context repo tomorrow if you don't get
> to it first.

offscreen_gl_context also do egl testing on linux. It seems to expect to use also EGL on some situations.

https://github.com/jrmuizel/gecko-dev/blob/Gecko-WebRenderer/third_party/rust/offscreen_gl_context/src/tests.rs#L6

Comment 4

[Sotaro Ikeda [:sotaro]]

(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> :kats how do you update the.cargo-checksum.json?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

(In reply to Sotaro Ikeda [:sotaro] from comment #4)
> (In reply to Sotaro Ikeda [:sotaro] from comment #2)
> > :kats how do you update the.cargo-checksum.json?

I did it by hand. I just ran sha256sum on the modified file and copied the hash into the .cargo-checksum.json file. That's ok for testing locally and try pushes, but we can't push that because the next time somebody runs |mach vendor rust| it will overwrite those changes. In this case we need to change offscreen_gl_context upstream, or fork it and put it into the gecko tree somewhere. I submitted a PR upstream to get the change landed:

https://github.com/emilio/rust-offscreen-rendering-context/pull/74

Comment 6

[Kartikaya Gupta (email:[e-mail])]

^ PR was merged. Waiting to get a new crate published on crates.io with the change before I close this bug.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/projects/graphics/rev/e2d01c06c77c26019d01e88b58e11f2ad48c7001
https://hg.mozilla.org/projects/graphics/rev/7d7fb34055316d6a93de8aeecfba1264db9b88d3

Comment 8

[Sotaro Ikeda [:sotaro]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #5)
> (In reply to Sotaro Ikeda [:sotaro] from comment #4)
> > (In reply to Sotaro Ikeda [:sotaro] from comment #2)
> > > :kats how do you update the.cargo-checksum.json?
> 
> I did it by hand. I just ran sha256sum on the modified file and copied the
> hash into the .cargo-checksum.json file. That's ok for testing locally and
> try pushes, but we can't push that because the next time somebody runs |mach
> vendor rust| it will overwrite those changes. In this case we need to change
> offscreen_gl_context upstream, or fork it and put it into the gecko tree
> somewhere. I submitted a PR upstream to get the change landed:
> 
> https://github.com/emilio/rust-offscreen-rendering-context/pull/74

Thanks for the info!