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 upSeparate FrameBuilder into it's own file #834
Conversation
This is the first step toward splitting up tiling.rs into logical units and will help to reduce merge conflicts in the future.
This can be part of the configuration structure instead of a free parameter.
|
I think the PR closes #587 |
|
@kvark It does help, but I still think there is more work to do. tiling.rs still contains many things that can be moved out. |
|
I'm not a fan of public-exposing everything, but otherwise looks good. |
| stacking_context_store: &'a [StackingContext], | ||
| prim_store: &'a PrimitiveStore, | ||
| resource_cache: &'a ResourceCache, | ||
| pub struct RenderTargetContext<'a> { |
This comment has been minimized.
This comment has been minimized.
kvark
Feb 6, 2017
Member
Can we have the fields hidden and only exposed to the module that uses them?
I imagine that both tiling and frame_builder need to create a RenderTargetContext, but do they both need to access the fields?
| clip_cache_info: Option<MaskCacheInfo>, | ||
| packed_layer_index: PackedLayerIndex, | ||
| xf_rect: Option<TransformedRect>, | ||
| pub scroll_layer_id: ScrollLayerId, |
This comment has been minimized.
This comment has been minimized.
kvark
Feb 6, 2017
Member
ok, so there is a lot of stuff that is now public
I suppose we can address the privacy concerns in follow-ups
This comment has been minimized.
This comment has been minimized.
mrobinson
Feb 7, 2017
Author
Member
I agree. I think once everything has been moved it makes sense to go back and reduce the visibility of everything by adding methods to the structs.
|
@bors-servo r+ |
|
|
Separate FrameBuilder into it's own file This is the first step toward splitting up tiling.rs into logical units and will help to reduce merge conflicts in the future. <!-- 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/834) <!-- Reviewable:end -->
|
|
|
Thanks for the reviews. |
mrobinson commentedFeb 6, 2017
•
edited by larsbergstrom
This is the first step toward splitting up tiling.rs into logical
units and will help to reduce merge conflicts in the future.
This change is