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

WebGL support on Windows #13840

Merged
merged 1 commit into from Oct 21, 2016
Merged

Conversation

@MortimerGoro
Copy link
Contributor

MortimerGoro commented Oct 19, 2016

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


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

Copy link
Member

emilio left a comment

Looks pretty good! Just a few nits :)

@@ -642,6 +662,10 @@ impl<Window: WindowMethods> IOCompositor<Window> {
}
}

(Msg::Dispatch(func), _) => {

This comment has been minimized.

@emilio

emilio Oct 19, 2016

Member

This second tuple member should be ShutdownState::NotShuttingDown.

@@ -642,6 +662,10 @@ impl<Window: WindowMethods> IOCompositor<Window> {
}
}

(Msg::Dispatch(func), _) => {
func();

This comment has been minimized.

@emilio

emilio Oct 19, 2016

Member

Could we add a comment here like:

// The functions sent here right now are really dumb, so they can't panic. But if we start running more complex code here, we should really catch panic here.

That should be enough for now. Also, how about renaming the Dispatch message to DispatchForWebGLContextCreation (or something like that)? I don't think we want this to become a general API.

I filled servo/surfman#72, somewhat related to error handling here.

This comment has been minimized.

@MortimerGoro

MortimerGoro Oct 20, 2016

Author Contributor

I think that might be useful in other future cases. The comments already say what it is used for now, but I'd bet that other API will require it too.

@@ -126,6 +126,8 @@ pub enum Msg {
// sends a reply on the IpcSender, the constellation knows it's safe to
// tear down the other threads associated with this pipeline.
PipelineExited(PipelineId, IpcSender<()>),
/// Runs a closure in the compositor thread

This comment has been minimized.

@emilio

emilio Oct 19, 2016

Member

A more elaborated comment here would be extremely valuable IMO :)

@emilio emilio self-assigned this Oct 19, 2016
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:webrender_dispatcher branch from e42326b to c79aeda Oct 20, 2016
@MortimerGoro
Copy link
Contributor Author

MortimerGoro commented Oct 20, 2016

@MortimerGoro MortimerGoro force-pushed the MortimerGoro:webrender_dispatcher branch from c79aeda to b1b074e Oct 20, 2016
@MortimerGoro
Copy link
Contributor Author

MortimerGoro commented Oct 20, 2016

#13843 is ready. Rebased ;)

@emilio
Copy link
Member

emilio commented Oct 21, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2016

📌 Commit b1b074e has been approved by emilio

@emilio
Copy link
Member

emilio commented Oct 21, 2016

@MortimerGoro If you have a screenshot I'll try to make sure it appears featured in TWIS :-)

@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2016

Testing commit b1b074e with merge 4216224...

bors-servo added a commit that referenced this pull request Oct 21, 2016
WebGL support on Windows

<!-- Please describe your changes on the following line: -->
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 _____

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13840)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Oct 21, 2016

@bors-servo bors-servo merged commit b1b074e into servo:master Oct 21, 2016
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Oct 21, 2016
4 of 5 tasks complete
@MortimerGoro
Copy link
Contributor Author

MortimerGoro commented Oct 21, 2016

@emilio Awesome, here is a screenshot of a WebGL animation. We could also mention that the WebVR API is coming to Servo. I've working on it and already have a core component that works outside servo. The next milestone is integrating it into servo. We'll publish a blogpost about that in mozvr blog in 1-2 days.

webgl_windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.