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 upOnly request tiles which have out-of-date content #86
Conversation
| let ContentAge(ref mut u) = *self; | ||
| *u += 1; | ||
| } | ||
| } |
This comment has been minimized.
This comment has been minimized.
zwarich
Jul 17, 2014
Contributor
Does it make sense for this to be a tuple struct where everything is public? That sort of takes away the benefits of the abstraction. As far as I can tell, other modules only ever create a new ContentAge starting at zero and then compare them. Maybe it should be a struct with a constructor that starts it out at zero.
This comment has been minimized.
This comment has been minimized.
mrobinson
Jul 17, 2014
Author
Member
I'll give your suggestion a try. The implementation is currently just a copy-and-paste (obviously not ideal) of Epoch from Servo.
| // a stale buffer at some point. We could handle this situation more gracefully if | ||
| // Servo and rust-layers shared Epoch/ContentAge values. That would make using rust-layers | ||
| // more complicated though. | ||
| match self.content_age_of_pending_buffer { |
This comment has been minimized.
This comment has been minimized.
zwarich
Jul 17, 2014
Contributor
Do you think that Servo and rust-layers should share Epoch/ContentAge values? In theory that makes more sense to me, since there are fewer possibilities for mismatched views of the flow of time where one is incremented yet the other is not.
This comment has been minimized.
This comment has been minimized.
mrobinson
Jul 17, 2014
Author
Member
I went back and forth on this quite a bit. My original thought was that they could be shared, but that means that any application using rust-layers would need to manage Epochs itself (like servo). That kind of interface might be too complicated for every application.
Currently the contract is that the application/embedding library must call Layer::content_changed when the surface content changes. If it calls content_changed twice before adding buffers, it should call add_buffers once with the latest contents. Adding two sets of buffers will trigger this warning.
I believe we should think carefully about a way to keep the interface simple between the two layers, while still properly handling receiving old buffers.
This comment has been minimized.
This comment has been minimized.
zwarich
Jul 17, 2014
Contributor
I think your concerns are reasonable, but in general we shouldn't favor a simpler interface for hypothetical secondary clients of rust-layers if it hurts the readability / enforced correctness of Servo code. Having the interface split is still very helpful even if we don't have any other clients, though.
I recommend we just discuss this at some point after this PR.
| } | ||
|
|
||
| // Don't resend a request, if we already have one pending. | ||
| return match self.content_age_of_pending_buffer { |
This comment has been minimized.
This comment has been minimized.
| } | ||
| } | ||
| pub fn next(&mut self) { | ||
| self.age = self.age + 1; |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Instead of constantly requesting tiles and throttling requests, only request tiles if the application signals that the layer contents have changed.
Only request tiles which have out-of-date content
mrobinson commentedJul 17, 2014
Instead of constantly requesting tiles and throttling requests, only
request tiles if the application signals that the layer contents have
changed.