Implement GLContextDispatcher trait for NativeGLContexts #432

MortimerGoro · 2016-10-05T22:55:15Z

Implement GLContextDispatcher trait for NativeGLContexts. Required to allow GLContext sharing in some implementations like WGL.

See servo/surfman#69 for more info.

This change is

emilio · 2016-10-05T22:56:10Z

webrender/src/render_backend.rs

@@ -42,6 +43,7 @@ pub struct RenderBackend {
    webgl_contexts: HashMap<WebGLContextId, GLContextWrapper>,
    current_bound_webgl_context_id: Option<WebGLContextId>,
    enable_recording: bool,
+    main_thread_dispatcher: Arc<Mutex<Option<Box<RenderDispatcher>>>>


Probably an option of an arc is a better Idea?

I used the same type pattern as the one used in RenderNotifier setter

Hmm... I see why you need this, but it seems unfortunate.

emilio · 2016-10-05T22:56:48Z

webrender/src/render_backend.rs

@@ -261,7 +265,10 @@ impl RenderBackend {
                        }
                        ApiMsg::RequestWebGLContext(size, attributes, tx) => {
                            if let Some(ref wrapper) = self.webrender_context_handle {
-                                let result = wrapper.new_context(size, attributes);
+                                let dispatcher = Box::new(WebRenderGLDispatcher {


Do this only on windows?

Yeah, I'll add a macro. It will avoid unneeded allocations in the platforms that don't use this.

emilio · 2016-10-05T22:58:59Z

I need to review this really carefully. May you open also a PR with your servo-related changes so it's easy to see the code flow?

MortimerGoro · 2016-10-05T23:11:50Z

Still have to rebase the latest servo master, I'll do tomorrow. In the meantime you can check this: MortimerGoro/servo@64f92be, it will be the same code, just changing Dispatcher name to RenderDispatcher.

P.D.: I created another trait called RenderDispatcher in order to not expose internal types of webrender. I assumed that you want that webrender library users don't need to be aware that offscreen_gl_context is used internally. Servo now uses offscreen_gl_context includes, but I thought that might not be true in the future when webrender is set up as default and skia renderer dropped.

bors-servo · 2016-10-11T04:24:31Z

☔ The latest upstream changes (presumably #439) made this pull request unmergeable. Please resolve the merge conflicts.

MortimerGoro · 2016-10-18T10:27:49Z

Any news on this?

I have a another idea to implement this. We could create and pass the dispatcher directly from WebGLPaintThred in servo, in this call.

wr_api.request_webgl_context(&size, attrs)

Let me know what you think

emilio

Sorry for the huge delay.

The other idea is nice too, but I think one global dispatcher is fine for now. Looks good to me with that nit addressed, and optionally that comment moved.

Thanks, and sorry again it took so long :)

emilio · 2016-10-18T20:22:02Z

webrender/src/render_backend.rs

-                                let result = wrapper.new_context(size, attributes);
+                                let dispatcher: Option<Box<GLContextDispatcher>>;
+                                if cfg!(target_os = "windows") {
+                                    dispatcher = Some(Box::new(WebRenderGLDispatcher {


nit:

let dispatcher = if cfg!(target_os = "windows") { Some(...) } else { None };

I had to specify the type: Option<Box<GLContextDispatcher>> or rust compiler doesn't have enough magic to coerce

emilio · 2016-10-18T20:22:02Z

webrender/src/renderer.rs

+    /// Sets the new MainThreadDispatcher.
+    ///
+    /// Used to dispatch functions to the main thread's event loop.
+    /// Required to allow GLContext sharing in some implementations like WGL.


probably moving the last part of this comment to the main_thread_dispatcher member definition would make it a bit more helpful.


        Implement GLContextDispatcher trait for NativeGLContexts, r=emilio

MortimerGoro · 2016-10-18T22:12:06Z

Done :)

emilio · 2016-10-18T22:24:40Z

@bors-servo: r+

bors-servo · 2016-10-18T22:24:43Z

📌 Commit 027e224 has been approved by emilio

bors-servo · 2016-10-18T22:24:45Z

⌛ Testing commit 027e224 with merge b667e13...


        Auto merge of #432 - MortimerGoro:dispatcher, r=emilio

Implement GLContextDispatcher trait for NativeGLContexts Implement GLContextDispatcher trait for NativeGLContexts. Required to allow GLContext sharing in some implementations like WGL. See servo/surfman#69 for more info.  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/432)

bors-servo · 2016-10-18T23:06:27Z

☀️ Test successful - status-travis


        Auto merge of #13840 - MortimerGoro:webrender_dispatcher, r=emilio

WebGL support on Windows  This is the final step to provide WebGL support on Windows ;) Some Related PRs already merged in webrender and offscreen-gl-context: servo/surfman#64 servo/webrender#432 --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____  --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13840)


        servo: Merge #13840 - WebGL support on Windows (from MortimerGoro:web…

…render_dispatcher); r=emilio  This is the final step to provide WebGL support on Windows ;) Some Related PRs already merged in webrender and offscreen-gl-context: servo/surfman#64 servo/webrender#432 --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ Source-Repo: https://github.com/servo/servo Source-Revision: 4216224f9cc7f3430db2b59f4d77061824ba7a1e


        servo: Merge #13840 - WebGL support on Windows (from MortimerGoro:web…

…render_dispatcher); r=emilio  This is the final step to provide WebGL support on Windows ;) Some Related PRs already merged in webrender and offscreen-gl-context: servo/surfman#64 servo/webrender#432 --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ Source-Repo: https://github.com/servo/servo Source-Revision: 4216224f9cc7f3430db2b59f4d77061824ba7a1e UltraBlame original commit: c57538159933d9eb1840437dfa9c9860220dd805


        servo: Merge #13840 - WebGL support on Windows (from MortimerGoro:web…

…render_dispatcher); r=emilio  This is the final step to provide WebGL support on Windows ;) Some Related PRs already merged in webrender and offscreen-gl-context: servo/surfman#64 servo/webrender#432 --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ Source-Repo: https://github.com/servo/servo Source-Revision: 4216224f9cc7f3430db2b59f4d77061824ba7a1e UltraBlame original commit: c57538159933d9eb1840437dfa9c9860220dd805


        servo: Merge #13840 - WebGL support on Windows (from MortimerGoro:web…

…render_dispatcher); r=emilio  This is the final step to provide WebGL support on Windows ;) Some Related PRs already merged in webrender and offscreen-gl-context: servo/surfman#64 servo/webrender#432 --- - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [ ] These changes fix #__ (github issue number if applicable). - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____ Source-Repo: https://github.com/servo/servo Source-Revision: 4216224f9cc7f3430db2b59f4d77061824ba7a1e UltraBlame original commit: c57538159933d9eb1840437dfa9c9860220dd805

MortimerGoro changed the title ~~Implement GLContextDispatcher trait for NativeGLContexts, r=emilio~~ Implement GLContextDispatcher trait for NativeGLContexts Oct 5, 2016

emilio reviewed Oct 5, 2016

View changes

MortimerGoro force-pushed the MortimerGoro:dispatcher branch from 80f97b8 to 62aaa20 Oct 7, 2016

MortimerGoro force-pushed the MortimerGoro:dispatcher branch 2 times, most recently from 3cf4b78 to a189acc Oct 13, 2016

emilio approved these changes Oct 18, 2016

View changes

Implement GLContextDispatcher trait for NativeGLContexts, r=emilio

Loading status checks…

027e224

MortimerGoro force-pushed the MortimerGoro:dispatcher branch from a189acc to 027e224 Oct 18, 2016

bors-servo merged commit 027e224 into servo:master Oct 18, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

This was referenced Oct 18, 2016

Switch primitives to be stored in SoA style in vertex textures. #448

Merged

Image mask support #445

Merged

MortimerGoro mentioned this pull request Oct 19, 2016

WebGL support on Windows servo/servo#13840

Merged

3 of 5 tasks complete

servo / webrender

Implement GLContextDispatcher trait for NativeGLContexts #432

Implement GLContextDispatcher trait for NativeGLContexts #432

MortimerGoro commented Oct 5, 2016 •

edited by larsbergstrom

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

emilio commented Oct 5, 2016

MortimerGoro commented Oct 5, 2016 •

edited

bors-servo commented Oct 11, 2016

MortimerGoro commented Oct 18, 2016

emilio left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

MortimerGoro commented Oct 18, 2016

emilio commented Oct 18, 2016

bors-servo commented Oct 18, 2016

bors-servo commented Oct 18, 2016

bors-servo commented Oct 18, 2016

servo / webrender

Join GitHub today

Implement GLContextDispatcher trait for NativeGLContexts #432

Implement GLContextDispatcher trait for NativeGLContexts #432

Conversation

MortimerGoro commented Oct 5, 2016 • edited by larsbergstrom

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

emilio commented Oct 5, 2016

MortimerGoro commented Oct 5, 2016 • edited

bors-servo commented Oct 11, 2016

MortimerGoro commented Oct 18, 2016

emilio left a comment

This comment has been minimized.

This comment has been minimized.

MortimerGoro Oct 18, 2016 • edited

This comment has been minimized.

MortimerGoro commented Oct 18, 2016

emilio commented Oct 18, 2016

bors-servo commented Oct 18, 2016

bors-servo commented Oct 18, 2016

bors-servo commented Oct 18, 2016

MortimerGoro commented Oct 5, 2016 •

edited by larsbergstrom

MortimerGoro commented Oct 5, 2016 •

edited

MortimerGoro Oct 18, 2016 •

edited