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 upMake image mask path optional in YAML reader #2225
Conversation
|
Is this what was occurring when converting a binary replay to yaml? I think there might be a deeper bug in the binary writer not correctly storing image mask paths. Is it actually useful to support image masks without a path? |
yes
would be great to look deeper into that for sure
not much: my patch makes the reader ignore such masks |
|
I'm not sure this is super useful - but r=me if it helps in the interim until we fix the bug in the yaml writer. We should probably log a warning that an invalid image mask was found and dropped though? |
|
Perhaps this is happening when images are provided via data URLs? In any case, I think it's good to be robust here. Looks good to me, with a couple small cleanups. |
| @@ -464,7 +464,19 @@ impl YamlFrameReader { | |||
| return None; | |||
| } | |||
|
|
|||
| let file = rsrc_path(&item["image"], &self.aux_dir); | |||
| //rsrc_path(&item["image"], &self.aux_dir); | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| file | ||
| } | ||
| None => { | ||
| //warn!("No image provided for the image-mask!"); |
This comment has been minimized.
This comment has been minimized.
mrobinson
Dec 15, 2017
Member
This warning looks useful, but if you still think it doesn't belong, might as well remove the line.
This comment has been minimized.
This comment has been minimized.
kvark
Dec 15, 2017
Author
Member
commented out because wrench isn't hooked up to log. I suppose this fixable :)
This comment has been minimized.
This comment has been minimized.
mrobinson
Dec 15, 2017
Member
Hrm. I notice that when I run the ahem.yaml reftest I get this output:
REFTEST reftests/text/ahem.yaml == reftests/text/ahem-ref.yaml
Skipping unknown item type: Hash({String("bounds"): Array([Integer(0), Integer(0), Integer(0), Integer(0)]), String("clip-rect"): Array([Integer(0), Integer(0), Integer(0), Integer(0)]), String("clip-and-scroll"): Integer(0), String("backface-visible"): Boolean(true)})
Skipping unknown item type: Hash({String("bounds"): Array([Integer(0),
|
@kvark What's the status of this one? |
|
@glennw updated and ready for merging now |
|
@bors-servo r+ |
|
|
Make image mask path optional in YAML reader Otherwise we crash during "show" command on some frames. Appears to be broken as of #1412 <!-- 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/2225) <!-- Reviewable:end -->
|
|
kvark commentedDec 14, 2017
•
edited
Otherwise we crash during "show" command on some frames.
Appears to be broken as of #1412
This change is