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

Add IOSurface ColorAttachment kind #137

Merged
merged 2 commits into from Jul 24, 2019
Merged

Add IOSurface ColorAttachment kind #137

merged 2 commits into from Jul 24, 2019

Conversation

@zakorgy
Copy link
Contributor

zakorgy commented Jun 4, 2019

Use IOSurfaces bound to textures as Framebuffer attachments, based on #131.
We have three textures bound to IOSurfaces, one active (bound to the context), one complete and one published to the WR thread. I found that using three IOSurfaces was the most more appropriate solution.
The body of reset_draw_buffer_contents is empty because that caused wrong background colors with servo/servo#23509, I still need to figure out that part.


This change is Reviewable

self.gl().tex_parameter_i(gl::TEXTURE_RECTANGLE, gl::TEXTURE_WRAP_S, gl::CLAMP_TO_EDGE as GLint);
self.gl().tex_parameter_i(gl::TEXTURE_RECTANGLE, gl::TEXTURE_WRAP_T, gl::CLAMP_TO_EDGE as GLint);

self.gl().bind_texture(gl::TEXTURE_2D, 0);

This comment has been minimized.

Copy link
@jdm

jdm Jun 4, 2019

Member

This should be TEXTURE_RECTANGLE.

Copy link
Member

jdm left a comment

This looks ok, but we need to figure out the issue with reset_draw_buffer.

src/draw_buffer.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
src/draw_buffer.rs Outdated Show resolved Hide resolved
src/draw_buffer.rs Outdated Show resolved Hide resolved
src/draw_buffer.rs Show resolved Hide resolved
src/draw_buffer.rs Show resolved Hide resolved
@zakorgy
Copy link
Contributor Author

zakorgy commented Jun 11, 2019

@jdm I made the proposed changes, also changed the swap logic according to servo/servo#23509 (comment). I still need to look into the reset_draw_buffer issue.

@zakorgy zakorgy force-pushed the zakorgy:io-surface branch from 8a161aa to 863cd92 Jun 11, 2019
@zakorgy
Copy link
Contributor Author

zakorgy commented Jun 11, 2019

Adding the clear_color value to reset_draw_buffer_contents fixes the background issue. Since each WebGL context in Servo tracks his own clear_color value in GLState we can pass it as a parameter when swapping buffers.

@jdm
jdm approved these changes Jun 11, 2019
Copy link
Member

jdm left a comment

We'll need to increase the Cargo version to 0.23 as well.

src/gl_context.rs Outdated Show resolved Hide resolved
src/draw_buffer.rs Outdated Show resolved Hide resolved
@zakorgy zakorgy force-pushed the zakorgy:io-surface branch from 278d5cd to 70b1633 Jul 17, 2019
@zakorgy
Copy link
Contributor Author

zakorgy commented Jul 17, 2019

This PR now depends on servo/core-foundation-rs#324

@zakorgy zakorgy force-pushed the zakorgy:io-surface branch 2 times, most recently from 7953725 to 4d36730 Jul 24, 2019
@zakorgy
Copy link
Contributor Author

zakorgy commented Jul 24, 2019

@jdm I have updated the PR and increased the crate version. Shall I squash the changes in one commit?

@jdm
Copy link
Member

jdm commented Jul 24, 2019

Yes, go ahead.

@zakorgy zakorgy force-pushed the zakorgy:io-surface branch from 4d36730 to 165d683 Jul 24, 2019
@zakorgy
Copy link
Contributor Author

zakorgy commented Jul 24, 2019

Squashed everything except the version bump commit.

@jdm
Copy link
Member

jdm commented Jul 24, 2019

@jdm jdm merged commit 92f0e69 into servo:master Jul 24, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@zakorgy zakorgy deleted the zakorgy:io-surface branch Jul 24, 2019
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

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