Svg thumbnail failed rendering #32936
Conversation
There was a problem hiding this comment.
I think the implementation should be changed, since I don't understand why JavaScript is needed to get the fill-rule. We're supposed to have that already from elsewhere.
Also this fixes for the svg thumbnail but not for svg preview. What about Peek? Does it have the same issue?
| @@ -120,7 +120,26 @@ public Bitmap GetThumbnailImpl(uint cx) | |||
| var a = await _browser.ExecuteScriptAsync($"document.getElementsByTagName('svg')[0].viewBox;"); | |||
| if (a != null) | |||
| { | |||
| await _browser.ExecuteScriptAsync($"document.getElementsByTagName('svg')[0].style = 'width:100%;height:100%';"); | |||
| // Retrieve the entire HTML content | |||
| var htmlContent = await _browser.ExecuteScriptAsync("document.documentElement.outerHTML;"); | |||
There was a problem hiding this comment.
Why do we have to run JavaScript to get these contents?
Aren't we supposed to have that already from other parts in the code? Thinking SvgContents.
Makes no sense to asynchronously run JavaScript to get something we should already have.
There was a problem hiding this comment.
Done.
This issue was not observed in svg preview.
But the same error was seen in Peek. Since a common structure is not used, it would be more accurate to examine it in a separate PR.
There was a problem hiding this comment.
Looking at the solution, it doesn't seem like a safe solution. According to the sample in the documentation linked in this PR ( https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill-rule ) , fill-rule might be present in internal polygons and not even the svg tag. So this change might actually apply an internal fill rule to the whole SVG, since it's applying it to the svg tag.
This is likely to create errors in trying to show others files.
If what we're trying to do is restore the "fill-rule" we seem to be removing from the style of the svg, perhaps the correct solution is edit or add just the width and height instead of completely overwriting the style of the svg tag like we were doing before.
Thoughts?
…iciently - Add retry logic and error handling for WebView2 initialization
|
Is this ready for review again? |
Summary of the Pull Request
Default value of fill-rule in style is "nonzero". There is a possibility that rendering bugs may occur due to this reason. Therefore, to eliminate these errors, it is checked whether there is a fill-rule in style and if so, it is re-applied.
https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill-rule
PR Checklist
Detailed Description of the Pull Request / Additional comments
I created some SVG files for testing. The first one is the "fill-rule" attribute is under style. In others it's under "polygon" and "path". When I tested it with version 0.84.1, only the one under style was broken. Therefore, I was able to solve the problem by parsing the svg style of the file and re-applying the "fill-rule" and other attributes there. The reason why I re-applied all the attributes in the style was that if there was such an error in the "fill-rule", I suspected that there might be other attributes as well. When I changed the "stroke color", "clip-rule", "stroke-width", "opacity" attributes, I did not encounter this error. However, I preserved the code this way as similar errors may occur in other attributes in the future.
0.84.1
PR
Validation Steps Performed
Check with these svg files
Test files.zip