Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAllow to not depend on offscreen_gl_context. #773
Conversation
|
Looks sensible to me. Just need to fix the CI and maybe answer my concern about the dead code. |
| @@ -186,6 +186,7 @@ pub struct ResourceCache { | |||
| cached_images: ResourceClassCache<ImageRequest, CachedImageInfo>, | |||
|
|
|||
| // TODO(pcwalton): Figure out the lifecycle of these. | |||
| #[cfg_attr(not(feature = "webgl"), allow(dead_code))] | |||
This comment has been minimized.
This comment has been minimized.
kvark
Jan 24, 2017
Member
why do we need to allow all these dead_code pieces instead of completely removing them?
This comment has been minimized.
This comment has been minimized.
emilio
Jan 24, 2017
Author
Member
We can remove them indeed, but overall in big structs, I didn't think it is worth to duplicate all the constructors just to avoid that field. I can do it if you prefer it that way, I just thought adding fields would become a bit more annoying.
This PR allows (via feature-gate) to compile out enough WebGL-related code in order to not depend in offscreen_gl_context. Fixes bug 1331546 Fixes servo/surfman#77
|
Let me know if you want me to do a followup removing those fields. @bors-servo r=kvark |
|
|
Allow to not depend on offscreen_gl_context. This PR allows (via feature-gate) to compile out enough WebGL-related code in order to not depend in offscreen_gl_context. Fixes bug 1331546 Fixes servo/surfman#77 r? @glennw or @kvark cc @sotaroikeda <!-- 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/773) <!-- Reviewable:end -->
|
|
|
Had to work around serde-rs/serde#720. Isn't ultra-pretty, but didn't think of a better way. Mind reviewing the last commit @kvark? |
|
@emilio This appears to have compile errors on CI. |
|
Gah, of course, this builds fine with derive + WebGL (Servo's configuration), and codegen - WebGL (Gecko's), but none of those are the default. @sotaroikeda, if this is not hi-priority, we could wait a few weeks until rust stable supports custom derive (it's on beta right now), are you fine with that? |
| // Some stubs to work around https://github.com/serde-rs/serde/issues/720 | ||
| #[cfg(not(feature = "webgl"))] | ||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| pub struct GLContextAttributes([u8; 0]); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
emilio
Jan 26, 2017
Author
Member
Apparently it's the sound way of making opaque enums that can't be instantiated, see rust-lang/rfcs#1861 (comment)
|
|
|
@emilio @sotaroikeda Have we decided to hold off on this until the rustc support lands? |
|
Yes, I believe so, unless it's super-hi-pri for Gecko, it's going to be cleaner than adding a lots of workarounds and then removing them shortly after. |
|
Yes, I'll rebase that as soon as I have the time.
…On Tue, Feb 28, 2017 at 07:43:41PM -0800, sotaroikeda wrote:
@emilio, can we do it cleaner with #941?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#773 (comment)
|
Allow to not depend on offscreen_gl_context It is created based on #773. cc @emilio, @kvark, @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/1009) <!-- Reviewable:end -->
emilio commentedJan 24, 2017
•
edited by larsbergstrom
This PR allows (via feature-gate) to compile out enough WebGL-related code in
order to not depend in offscreen_gl_context.
Fixes bug 1331546
Fixes servo/surfman#77
r? @glennw or @kvark
cc @sotaroikeda
This change is