Closed
Bug 1495462
Opened 6 years ago
Closed 6 years ago
Add a mechanism to get a WR capture on a reftest
Categories
(Core :: Graphics: WebRender, enhancement, P2)
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: kats, Assigned: kats)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(2 files)
5.67 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
1.49 KB,
patch
|
jrmuizel
:
review+
|
Details | Diff | Splinter Review |
I have patches for this from a while back that I needed to use again, so it's probably worth landing them into the tree.
Assignee | ||
Comment 1•6 years ago
|
||
Attachment #9013342 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 2•6 years ago
|
||
Attachment #9013346 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 3•6 years ago
|
||
Actually I should move the print at [1] into bindings.rs as well and have it emit the corrected full path. [1] https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/gfx/webrender_bindings/WebRenderAPI.cpp#587
Comment 4•6 years ago
|
||
Comment on attachment 9013346 [details] [diff] [review] Part 2 - Avoid captures clobbering previous captures Review of attachment 9013346 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/webrender_bindings/src/bindings.rs @@ +1503,5 @@ > + let count = if let Some(old_extension) = path.extension() { > + old_extension.to_str().unwrap_or("0").parse::<u32>().unwrap_or(0) > + } else { > + 0 > + }; Do you like: let count: u32 = path.extension() .and(|x| x.to_str()) .map(|x| x.parse()) .unwrap_or(0) better? @@ +1504,5 @@ > + old_extension.to_str().unwrap_or("0").parse::<u32>().unwrap_or(0) > + } else { > + 0 > + }; > + path.set_extension(format!("{}", count + 1)); (count + 1).to_string()? is probably better.
Attachment #9013346 -
Flags: review?(jmuizelaar) → review+
Comment 5•6 years ago
|
||
Comment on attachment 9013342 [details] [diff] [review] Part 1 - Add a reftest.list annotation to trigger a capture Review of attachment 9013342 [details] [diff] [review]: ----------------------------------------------------------------- It'd be sort of nice if the name of the test made it into the name of capture directory. But this is fine.
Attachment #9013342 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #4) > Do you like: > let count: u32 = path.extension() > .and(|x| x.to_str()) > .map(|x| x.parse()) > .unwrap_or(0) > > better? I do! Except it doesn't work quite as you wrote it. I had to s/.and/.and_then/ and s/.parse()/.parse().ok()/ to make it work. But yeah, that's cleaner. > > + path.set_extension(format!("{}", count + 1)); > > (count + 1).to_string()? is probably better. Ok, will change.
Assignee | ||
Comment 7•6 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6) > I do! Except it doesn't work quite as you wrote it. I had to > s/.and/.and_then/ and s/.parse()/.parse().ok()/ to make it work. But yeah, > that's cleaner. Also s/map/and_then/. i.e. https://play.rust-lang.org/?gist=621237cdcfb02ba41c85fe9bacad5027&version=stable&mode=debug&edition=2015
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/174c7b333596 Add a mechanism for grabbing a WR capture of a reftest. r=jrmuizel https://hg.mozilla.org/integration/mozilla-inbound/rev/fdbc4d498652 Increment the capture path extension until we find a fresh path. r=jrmuizel
Updated•6 years ago
|
Blocks: stage-wr-trains
Priority: -- → P2
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/174c7b333596 https://hg.mozilla.org/mozilla-central/rev/fdbc4d498652
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in
before you can comment on or make changes to this bug.
Description
•