Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render crates #236

Merged
merged 9 commits into from Apr 11, 2019

Conversation

Projects
None yet
4 participants
@ceyusa
Copy link
Contributor

commented Apr 6, 2019

This PR creates two new crates inside gstreamer backend:

  • A render crate which only defines a trait for different renders for different OS
  • A render object helper that will setup the rendering bits for the player
  • A unix-render crate which implementes the render trait for Unix-based GL systems

This will fix the problem with cargo with features per target.

It is not well tested yet. And the GL implementation still has some missing bits.

I would like to propose a new create depending on glutin to extract the GL context and display for different platforms used by the possible gl renders.

@jdm

This comment has been minimized.

Copy link
Member

commented Apr 6, 2019

Rather than adding a glutin dependency to the media crates, I think we should define a trait that provides the necessary APIs. This can then be implemented by the servo embedder, which will allow us to more easily integrate with platforms where we don't use glutin (like Android and magic leap).

@ceyusa ceyusa force-pushed the ceyusa:render-crates branch 3 times, most recently from 7141f15 to 0f144f5 Apr 8, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Just tested the branch in Mac and Win10, and in the first works, in the last compiles, but I got a strange webrender error. Removing WIP.

@ceyusa ceyusa changed the title WIP: Render crates Render crates Apr 8, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

commented Apr 9, 2019

@ferjm: r?

@ferjm

ferjm approved these changes Apr 9, 2019

Copy link
Member

left a comment

\o/ Great work!

I know the current state of documentation for this crate is not very good, but I think this is complex enough to deserve a little bit of documentation ;)

Thanks for fixing this!

Show resolved Hide resolved backends/gstreamer/player.rs Outdated
Show resolved Hide resolved backends/gstreamer/player.rs Outdated
Show resolved Hide resolved backends/gstreamer/player.rs
Show resolved Hide resolved backends/gstreamer/render-unix/lib.rs
Show resolved Hide resolved backends/gstreamer/render-unix/lib.rs
Show resolved Hide resolved backends/gstreamer/render-unix/lib.rs Outdated
Show resolved Hide resolved backends/gstreamer/render-unix/lib.rs
Create a GStreamerRender helper object
This object is in charge of two tasks:

- Create the video sink
- Create the frames to push to the Renders

The purpose of this design is to handle GL by another create (in order
to workaround
servo/servo#22944 (comment)), and
that other object will proxy these tasks if available in the target
OS.

This patch removes temporarly the GL support in player.

@ceyusa ceyusa force-pushed the ceyusa:render-crates branch from 0f144f5 to 388ddf1 Apr 10, 2019

ceyusa added some commits Apr 5, 2019

Create a GStreamerRender trait
This new trait is implemented by the possible renders, mainly the
OpenGL-based ones, where the video sink and frames are build.

A dummy render is defined by default.
Implement GL Render for Unix-based systems
Right now only EGL is supported.

@ceyusa ceyusa force-pushed the ceyusa:render-crates branch from 388ddf1 to 337c003 Apr 11, 2019

@ceyusa

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2019

@ferjm: r?

@ferjm

This comment has been minimized.

Copy link
Member

commented Apr 11, 2019

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

📌 Commit 1c4365a has been approved by ferjm

@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

⌛️ Testing commit 1c4365a with merge a5a8e6e...

bors-servo added a commit that referenced this pull request Apr 11, 2019

Auto merge of #236 - ceyusa:render-crates, r=ferjm
 Render crates

This PR creates two new crates inside gstreamer backend:

- A render crate which only defines a trait for different renders  for different OS
- A render object helper that will setup the rendering bits for the player
- A unix-render crate which implementes the render trait for Unix-based GL systems

This will fix the problem with cargo with features per target.

It is not well tested yet. And the GL implementation still has some missing bits.

I would like to propose a new create depending on glutin to extract the GL context and display for different platforms used by the possible gl renders.
@bors-servo

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

☀️ Test successful - checks-travis
Approved by: ferjm
Pushing a5a8e6e to master...

@bors-servo bors-servo merged commit 1c4365a into servo:master Apr 11, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@ceyusa ceyusa deleted the ceyusa:render-crates branch Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.