Add Wrench to WebRender #585
Conversation
| ] | ||
|
|
||
| [profile.release] | ||
| debug = true |
There was a problem hiding this comment.
Does this have any effect on runtime performance? Should we just enable it when profiling?
| panic!("Can't layout simple ascii on this platform"); | ||
| } | ||
|
|
||
| pub trait WrenchThing { |
|
|
||
| impl WrenchThing for BinaryFrameReader { | ||
| fn do_frame(&mut self, wrench: &mut Wrench) -> u32 { | ||
| if self.do_frame(wrench) == false { |
There was a problem hiding this comment.
should we check for frame_num == 0 here as well?
| use webrender_traits::*; | ||
|
|
||
| mod wrench; | ||
| use wrench::{Wrench, WrenchThing}; |
There was a problem hiding this comment.
I think this is against rust style guidelines, where you are supposed to have all the use to follow all the mod things
| static ref BLACK_COLOR: ColorF = ColorF::new(0.0, 0.0, 0.0, 1.0); | ||
| } | ||
|
|
||
| pub static mut CURRENT_FRAME_NUMBER: u32 = 0; |
There was a problem hiding this comment.
Where is this getting used? I only see assignment.
|
|
||
| pub static mut CURRENT_FRAME_NUMBER: u32 = 0; | ||
|
|
||
| enum ThingKind { |
There was a problem hiding this comment.
again, I wish it had a more descriptive name, like FrameSource
| } | ||
|
|
||
| pub struct Wrench { | ||
| pub window: glutin::Window, |
There was a problem hiding this comment.
does it all have to be public?
I know this is not much important, given that it's not a used API but just a tool
wondering just out of interest
| .build().unwrap(); | ||
|
|
||
| unsafe { | ||
| window.make_current().ok(); |
There was a problem hiding this comment.
I think we should be using unwrap() here, because ok() will not crash, and our behavior would be undefined later on
| } | ||
|
|
||
| let gl_version = unsafe { | ||
| let data = CStr::from_ptr(gl::GetString(gl::VERSION) as *const _).to_bytes().to_vec(); |
There was a problem hiding this comment.
you can borrow some ideas from #584
| } | ||
|
|
||
| fn handle_rect(&self, wrench: &mut Wrench, clip_region: &ClipRegion, item: &Yaml) | ||
| { |
There was a problem hiding this comment.
opening bracket - against the code guidelines (sorry to be pedantic!)
| } | ||
|
|
||
| fn handle_image(&self, wrench: &mut Wrench, clip_region: &ClipRegion, item: &Yaml) | ||
| { |
|
☔ The latest upstream changes (presumably #592) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@vvuk Happy to merge this when you are ready - let me know when it's good to go. |
|
@bors-servo r+ |
|
📌 Commit 7be8661 has been approved by |
|
⚡ Test exempted - status |
Add Wrench to WebRender This moves Wrench to WebRender. It can now replay binary recordings (replaces the wr-replay tool, though I haven't removed that one), show frames from YAML, and write YAML while replaying a binary recording. The YAML stuff is very much WIP, but what's there works and can be built on top of. <!-- 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/585) <!-- Reviewable:end -->
This moves Wrench to WebRender. It can now replay binary recordings (replaces the wr-replay tool, though I haven't removed that one), show frames from YAML, and write YAML while replaying a binary recording. The YAML stuff is very much WIP, but what's there works and can be built on top of.
This change is