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 upImplement clear #673
Implement clear #673
Conversation
|
Will add a commit to update submodules when servo/rust-netsurfcss#19 and servo/rust-css#23 are merged. |
|
|
||
| noncontent_height = base.model.padding.top + base.model.padding.bottom + | ||
| base.model.border.top + base.model.border.bottom; | ||
| base.position.size.height = height + noncontent_height; | ||
|
|
||
| noncontent_height = noncontent_height + base.model.margin.top + base.model.margin.bottom; | ||
| noncontent_height = noncontent_height + clearance + base.model.margin.top + base.model.margin.bottom; |
This comment has been minimized.
This comment has been minimized.
metajack
Aug 5, 2013
Contributor
It feels wrong to have clearance included in the height. It now makes the variable name wrong, since this is no longer the noncontent height, but noncontent height plus some other things.
I'd like to figure out some better way to include clearance.
|
pinging @eric93 |
eric93
commented
Aug 5, 2013
|
Looks good to me. The only thought I had is that it may be worthwhile to memoize calls to clearance() by storing the values in the float context itself. We would need to update them whenever a float is added but reading them would be fast and simple. |
|
@eric93 any thoughts on if clearance should go in FlowData to address my comments? |
eric93
commented
Aug 5, 2013
|
@metajack I think we should avoid putting things in FlowData unless they must be stored. It seems to me that clearance does have to be added to the height at some point, so the question is whether it's okay to put it under noncontent_height. I don't really have a strong opinion either way on this. |
This comment has been minimized.
This comment has been minimized.
metajack
commented on 23874eb
Aug 5, 2013
|
r+ |
|
After thinking about this more I'm r+ing it. Great work! |
This comment has been minimized.
This comment has been minimized.
|
saw approval from metajack |
This comment has been minimized.
This comment has been minimized.
|
merging sanxiyn/servo/clear = 23874eb into auto |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
fast-forwarding master to auto = 63e7866 |
bors-servo
merged commit 23874eb
into
servo:master
BrendanEich
commented
Aug 5, 2013
|
Thanks, Seo! /be |
Categorize a few media tests into spec section directories
Remove clip_clear and rectangle_noblend from the clip batcher. Now that clip masks are stored in the red channel, we can just clear the intermediate targets to (1,0,0,0). This satisfies the requirement of clip masks being initialized to 1, while also allowing the alpha component to be initialized to 0 for visual render tasks that rely on it. <!-- 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/673) <!-- Reviewable:end -->
sanxiyn commentedAug 5, 2013
No description provided.