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 encapsulation #1530

Merged
merged 1 commit into from Jul 28, 2017
Merged

WebGL encapsulation #1530

merged 1 commit into from Jul 28, 2017

Conversation

@kvark
Copy link
Member

kvark commented Jul 28, 2017

Refactors our WebGL usage and fixes a regression from #1509
also related to #1353 (should make it easier)

r? @MortimerGoro
cc @glennw


This change is Reviewable

@MortimerGoro
Copy link
Contributor

MortimerGoro commented Jul 28, 2017

:lgtm:


Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@kvark
Copy link
Member Author

kvark commented Jul 28, 2017

not sure if this is going to work
@bors-servo r=MortimerGoro

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

📌 Commit 9d8bb20 has been approved by MortimerGoro

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

Testing commit 9d8bb20 with merge 1ec6fb9...

bors-servo added a commit that referenced this pull request Jul 28, 2017
WebGL encapsulation

Refactors our WebGL usage and fixes a regression from #1509
also related to #1353 (should make it easier)

r? @MortimerGoro
cc @glennw

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/1530)
<!-- Reviewable:end -->
@@ -105,6 +105,59 @@ impl Document {
}
}

struct WebGL {
last_id: WebGLContextId,
contexts: HashMap<WebGLContextId, GLContextWrapper, BuildHasherDefault<FnvHasher>>,

This comment has been minimized.

Copy link
@emilio

emilio Jul 28, 2017

Member

nit: Could use fnv::FnvHashMap, and HashMap::default() below.

This comment has been minimized.

Copy link
@kvark

kvark Jul 28, 2017

Author Member

I just followed the existing code. Would be great to convert it in bulk to a shorter form as you propose, just in a separate PR

This comment has been minimized.

Copy link
@MortimerGoro

MortimerGoro Jul 28, 2017

Contributor

The WebGL code in WR will be deleted after servo/servo#17891 lands, so it's not worth the effort to work on that if it only affects webgl (yes if it changes other parts of the code)

This comment has been minimized.

Copy link
@kvark

kvark Jul 28, 2017

Author Member

@MortimerGoro wow, +3,138 −1,417

@bors-servo
Copy link
Contributor

bors-servo commented Jul 28, 2017

☀️ Test successful - status-travis
Approved by: MortimerGoro
Pushing 1ec6fb9 to master...

@bors-servo bors-servo merged commit 9d8bb20 into servo:master Jul 28, 2017
3 of 4 checks passed
3 of 4 checks passed
code-review/reviewable 1 discussion left (emilio, MortimerGoro)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@kvark kvark deleted the kvark:webgl branch Jul 28, 2017
@kvark kvark mentioned this pull request Jul 28, 2017
2 of 5 tasks complete
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.