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

New GLLimits struct to access GL "limit values". #48

Merged
merged 1 commit into from Jan 9, 2016

Conversation

@simartin
Copy link
Contributor

simartin commented Jan 9, 2016

Implement suggestion in code review for my fix for servo issue #9097 at https://reviewable.io/reviews/servo/servo/9102#-K6p6P67OPgZjXKhbiD8

@@ -61,6 +65,7 @@ impl<Native> GLContext<Native>
let mut context = try!(Self::create(shared_with));

context.formats = GLFormats::detect(&attributes);
context.limits = GLLimits::detect();

This comment has been minimized.

Copy link
@emilio

emilio Jan 9, 2016

Member

There should be no need to re-detect the limits here.

The formats are needed because the new attributes don't have to be the old ones, but the limits should always be the same.

use gleam::gl::types::GLint;
use gleam::gl;

#[derive(Deserialize, Serialize)]

This comment has been minimized.

Copy link
@emilio

emilio Jan 9, 2016

Member

Probably want to derive Clone too here, to send it around without calling detect() again.

@emilio
Copy link
Member

emilio commented Jan 9, 2016

Left a few comments, but this looks good to me :)

The CI failures are due to: rust-lang/rust#30713

@simartin simartin force-pushed the simartin:servo_issue_9097 branch from 853011f to 644079e Jan 9, 2016
@simartin
Copy link
Contributor Author

simartin commented Jan 9, 2016

Thanks, PR updated accordingly

@emilio
Copy link
Member

emilio commented Jan 9, 2016

Great! LGTM

emilio added a commit that referenced this pull request Jan 9, 2016
New GLLimits struct to access GL "limit values".
@emilio emilio merged commit 3d717d8 into servo:master Jan 9, 2016
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@simartin simartin deleted the simartin:servo_issue_9097 branch Jan 9, 2016
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.