Closed Bug 1318201 Opened 3 years ago Closed 3 years ago

Remove link to EGL on linux

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54

People

(Reporter: sotaro, Assigned: kats)

References

(Blocks 1 open bug)

Details

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.
Blocks: webrender
Assignee: nobody → sotaro.ikeda.g
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
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?
Assignee: sotaro.ikeda.g → bugmail
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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
Flags: needinfo?
Flags: needinfo?
Version: 38 Branch → unspecified
(In reply to Sotaro Ikeda [:sotaro] from comment #2)
> :kats how do you update the.cargo-checksum.json?
Flags: needinfo?(bugmail)
(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
Flags: needinfo?(bugmail)
^ PR was merged. Waiting to get a new crate published on crates.io with the change before I close this bug.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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!
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.