Skip to content

/ surfman

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

Add support for packed depth stencil attachment. #97

Merged
merged 1 commit into from Apr 11, 2017
Merged

Add support for packed depth stencil attachment. #97

merged 1 commit into from Apr 11, 2017

Conversation

@MortimerGoro
Copy link
Collaborator

MortimerGoro commented Apr 10, 2017

Add support for packed depth stencil attachment. Fix incomplete framebuffer error on some platforms when using both depth and stencil attachments.
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:packed_depth_stencil branch from ab76951 to 470817e Apr 10, 2017
@kvark
kvark approved these changes Apr 10, 2017
View changes
@kvark
Copy link
Member

kvark commented Apr 10, 2017 •
edited

@emilio do you want to also have a look?

Edit: thanks @MortimerGoro ! It's great to finally support packed DS, should get more efficient for our stencil usage.
@emilio
emilio approved these changes Apr 10, 2017
View changes
Copy link
Member

emilio left a comment

Looks good % nits, thanks @MortimerGoro!

Also, thanks for taking a look @kvark :)
src/gl_context.rs
@@ -47,8 +48,10 @@ impl<Native> GLContext<Native>
};

try!(native_context.make_current());
let extensions = gl_.get_string(gl::EXTENSIONS);
let extensions: Vec<String> = extensions.split(&[',',' '][..]).map(|s| s.into()).collect();

This comment has been minimized.

Sign in to view
@emilio

emilio Apr 10, 2017

Member

Do you need to collect into a Vec<String>? I'd expect Vec<&str> to work.

This comment has been minimized.

Sign in to view
@MortimerGoro

MortimerGoro Apr 10, 2017

Author Collaborator

I didn't use Vec<&str> in this case because the String returned from gl.get_string(gl::EXTENSIONS) is destroyed. We'd need to save it somewhere to have a Vec<&str>

This comment has been minimized.

Sign in to view
@emilio

emilio Apr 11, 2017

Member

Oh, didn't notice you were saving all the extensions there, right. Then that looks fine.
src/gl_formats.rs
// detect if the GPU supports RGB8 and RGBA8 renderbuffer/texture storage formats.
// GL_ARM_rgba8 extension is similar to OES_rgb8_rgba8, but only exposes RGBA8.
let extensions = gl.get_string(gl::EXTENSIONS);
let extensions: Vec<&str> = extensions.split(&[',',' '][..]).collect();

This comment has been minimized.

Sign in to view
@emilio

emilio Apr 10, 2017

Member

Oh, so yeah, we can :)
src/tests.rs Outdated

#[test]
fn test_stencil_no_depth() {
let mut attributes = GLContextAttributes::default();

This comment has been minimized.

Sign in to view
@emilio

emilio Apr 10, 2017

Member

You can use the:

let attributes = GLContextAttributes {
    depth: true,
    stencil: true,
    .. Default::default(),
};

syntax, which is a bit nicer IMO.
@MortimerGoro
Add support for packed depth stencil attachment. Fix incomplete frame…
Loading status checks…
b983a0f 
…buffer error on some platforms when using both depth and stencil attachments.
@MortimerGoro MortimerGoro force-pushed the MortimerGoro:packed_depth_stencil branch from 470817e to b983a0f Apr 11, 2017
@MortimerGoro MortimerGoro merged commit df98de8 into servo:master Apr 11, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@MortimerGoro MortimerGoro mentioned this pull request May 15, 2017
Merged
3 of 5 tasks complete
bors-servo added a commit to servo/servo that referenced this pull request May 22, 2017
@bors-servo
Auto merge of #16867 - MortimerGoro:update_offscreen_gl_ctx, r=jdm
Loading status checks…
1799efe 
Update offscreen_gl_context to 0.8.8

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/16867)
<!-- Reviewable:end -->
bors-servo added a commit to servo/servo that referenced this pull request May 25, 2017
@bors-servo
Auto merge of #16867 - MortimerGoro:update_offscreen_gl_ctx, r=jdm
Loading status checks…
861a66e 
Update offscreen_gl_context to 0.8.8

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/16867)
<!-- Reviewable:end -->
bors-servo added a commit to servo/servo that referenced this pull request May 27, 2017
@bors-servo
Auto merge of #16867 - MortimerGoro:update_offscreen_gl_ctx, r=jdm
Loading status checks…
00a5da3 
Update offscreen_gl_context

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/16867)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 28, 2017
@MortimerGoro
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
a9cfdd9 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : fe30e163752773708e2fd1f00abe0662237cff24
aethanyc pushed a commit to aethanyc/gecko-dev that referenced this pull request May 28, 2017
@MortimerGoro
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
afcd9eb 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request May 30, 2017
@MortimerGoro
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
b0da3d7 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47
Manishearth pushed a commit to Manishearth/gecko-dev that referenced this pull request Jun 11, 2017
@MortimerGoro
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
1eedade 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47
brendandahl pushed a commit to brendandahl/gecko that referenced this pull request Jul 10, 2017
@MortimerGoro
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
74d8575 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 1, 2019
@marco-c
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
d449a0d 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47

UltraBlame original commit: 0e5adeceb9b8a1160c0a75989d571ca2928b7e58
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 1, 2019
@marco-c
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
333d80b 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47

UltraBlame original commit: 0e5adeceb9b8a1160c0a75989d571ca2928b7e58
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 1, 2019
@marco-c
servo: Merge #16867 - Update offscreen_gl_context (from MortimerGoro:…
8b21eeb 
…update_offscreen_gl_ctx); r=jdm

<!-- Please describe your changes on the following line: -->

Adds servo/surfman#97 and servo/surfman#98 required to run some WebGL demos

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [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).

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: 00a5da36fbe8fdc8336ff1800ad16d0141c77c47

UltraBlame original commit: 0e5adeceb9b8a1160c0a75989d571ca2928b7e58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio

@kvark kvark

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
@MortimerGoro @kvark @emilio
You can’t perform that action at this time.