From 16e105106bcc392c82d5b637e6daab4fa56f5512 Mon Sep 17 00:00:00 2001 From: Andrey Nekrasov Date: Thu, 7 Sep 2023 17:06:49 +0200 Subject: [PATCH] [SVGThumbnail]Optimize CPU usage (#28286) * [SVG Thumbnail] Optimize CPU usage * add missing zero --- .github/actions/spell-check/expect.txt | 1 - .../SvgThumbnailProvider.cs | 83 +++++--- .../SvgThumbnailProviderTests.cs | 94 +++++++++- .../Utilities/SvgPreviewHandlerHelper.cs | 177 +++++++++++++----- 4 files changed, 276 insertions(+), 79 deletions(-) diff --git a/.github/actions/spell-check/expect.txt b/.github/actions/spell-check/expect.txt index 69f51bbaf8..a4621083ce 100644 --- a/.github/actions/spell-check/expect.txt +++ b/.github/actions/spell-check/expect.txt @@ -2223,7 +2223,6 @@ Wubi WVC Wwan Wwanpp -XAttribute XAxis xbf Xbox diff --git a/src/modules/previewpane/SvgThumbnailProvider/SvgThumbnailProvider.cs b/src/modules/previewpane/SvgThumbnailProvider/SvgThumbnailProvider.cs index f8b8964be7..bd200c9aed 100644 --- a/src/modules/previewpane/SvgThumbnailProvider/SvgThumbnailProvider.cs +++ b/src/modules/previewpane/SvgThumbnailProvider/SvgThumbnailProvider.cs @@ -5,6 +5,7 @@ using System.Drawing.Drawing2D; using System.Globalization; using System.Reflection; using System.Runtime.CompilerServices; +using System.Threading; using Common.Utilities; using Microsoft.Web.WebView2.Core; using Microsoft.Web.WebView2.WinForms; @@ -35,6 +36,13 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg /// public Stream Stream { get; private set; } + /// + /// Gets or sets signalled when the main thread can use preprocessed svg contents. + /// + public ManualResetEventSlim SvgContentsReady { get; set; } = new ManualResetEventSlim(false); + + public string SvgContents { get; set; } = string.Empty; + /// /// The maximum dimension (width or height) thumbnail we will generate. /// @@ -87,20 +95,19 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg /// Render SVG using WebView2 control, capture the WebView2 /// preview and create Bitmap out of it. /// - /// The content to render. /// The maximum thumbnail size, in pixels. - public Bitmap GetThumbnail(string content, uint cx) + public Bitmap GetThumbnailImpl(uint cx) { CleanupWebView2UserDataFolder(); - if (cx == 0 || cx > MaxThumbnailSize || string.IsNullOrEmpty(content) || !content.Contains("svg")) + if (cx == 0 || cx > MaxThumbnailSize) { return null; } Bitmap thumbnail = null; - bool thumbnailDone = false; - string wrappedContent = WrapSVGInHTML(content); + + var thumbnailDone = new ManualResetEventSlim(false); _browser = new WebView2(); _browser.Dock = DockStyle.Fill; @@ -132,7 +139,7 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg thumbnail = ResizeImage(thumbnail, scaleWidth, scaleHeight); } - thumbnailDone = true; + thumbnailDone.Set(); }; var webView2Options = new CoreWebView2EnvironmentOptions("--block-new-web-contents"); @@ -140,6 +147,7 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg webView2EnvironmentAwaiter = CoreWebView2Environment .CreateAsync(userDataFolder: _webView2UserDataFolder, options: webView2Options) .ConfigureAwait(true).GetAwaiter(); + webView2EnvironmentAwaiter.OnCompleted(async () => { try @@ -170,16 +178,23 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg // WebView2.NavigateToString() limitation // See https://learn.microsoft.com/dotnet/api/microsoft.web.webview2.core.corewebview2.navigatetostring?view=webview2-dotnet-1.0.864.35#remarks // While testing the limit, it turned out it is ~1.5MB, so to be on a safe side we go for 1.5m bytes - if (wrappedContent.Length > 1_500_000) + SvgContentsReady.Wait(); + if (string.IsNullOrEmpty(SvgContents) || !SvgContents.Contains("svg")) + { + thumbnailDone.Set(); + return; + } + + if (SvgContents.Length > 1_500_000) { string filename = _webView2UserDataFolder + "\\" + Guid.NewGuid().ToString() + ".html"; - File.WriteAllText(filename, wrappedContent); + File.WriteAllText(filename, SvgContents); _localFileURI = new Uri(filename); _browser.Source = _localFileURI; } else { - _browser.NavigateToString(wrappedContent); + _browser.NavigateToString(SvgContents); } } catch (Exception) @@ -187,7 +202,7 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg } }); - while (thumbnailDone == false) + while (!thumbnailDone.Wait(75)) { Application.DoEvents(); } @@ -276,31 +291,41 @@ namespace Microsoft.PowerToys.ThumbnailHandler.Svg return null; } - string svgData = null; - using (var reader = new StreamReader(this.Stream)) + if (Stream != null) { - svgData = reader.ReadToEnd(); - try + new Thread(() => { - // Fixes #17527 - Inkscape v1.1 swapped order of default and svg namespaces in svg file (default first, svg after). - // That resulted in parser being unable to parse it correctly and instead of svg, text was previewed. - // MS Edge and Firefox also couldn't preview svg files with mentioned order of namespaces definitions. - svgData = SvgPreviewHandlerHelper.SwapNamespaces(svgData); - svgData = SvgPreviewHandlerHelper.AddStyleSVG(svgData); - } - catch (Exception) - { - } + string svgData = null; + using (var reader = new StreamReader(Stream)) + { + svgData = reader.ReadToEnd(); + try + { + // Fixes #17527 - Inkscape v1.1 swapped order of default and svg namespaces in svg file (default first, svg after). + // That resulted in parser being unable to parse it correctly and instead of svg, text was previewed. + // MS Edge and Firefox also couldn't preview svg files with mentioned order of namespaces definitions. + svgData = SvgPreviewHandlerHelper.SwapNamespaces(svgData); + svgData = SvgPreviewHandlerHelper.AddStyleSVG(svgData); + SvgContents = WrapSVGInHTML(svgData); + SvgContentsReady.Set(); + } + catch (Exception) + { + SvgContentsReady.Set(); + } + } + }).Start(); + } + else + { + SvgContentsReady.Set(); } - if (svgData != null) + using (Bitmap thumbnail = GetThumbnailImpl(cx)) { - using (Bitmap thumbnail = GetThumbnail(svgData, cx)) + if (thumbnail != null && thumbnail.Size.Width > 0 && thumbnail.Size.Height > 0) { - if (thumbnail != null && thumbnail.Size.Width > 0 && thumbnail.Size.Height > 0) - { - return (Bitmap)thumbnail.Clone(); - } + return (Bitmap)thumbnail.Clone(); } } diff --git a/src/modules/previewpane/UnitTests-SvgThumbnailProvider/SvgThumbnailProviderTests.cs b/src/modules/previewpane/UnitTests-SvgThumbnailProvider/SvgThumbnailProviderTests.cs index 8af94dc6db..4e50e765c3 100644 --- a/src/modules/previewpane/UnitTests-SvgThumbnailProvider/SvgThumbnailProviderTests.cs +++ b/src/modules/previewpane/UnitTests-SvgThumbnailProvider/SvgThumbnailProviderTests.cs @@ -28,7 +28,9 @@ namespace SvgThumbnailProviderUnitTests svgBuilder.AppendLine(""); SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(svgBuilder.ToString(), 256); + svgThumbnailProvider.SvgContents = svgBuilder.ToString(); + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); Assert.IsNotNull(thumbnail); Assert.IsTrue(thumbnail.Width > 0); @@ -46,7 +48,73 @@ namespace SvgThumbnailProviderUnitTests svgBuilder.AppendLine(""); SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(svgBuilder.ToString(), 256); + svgThumbnailProvider.SvgContents = svgBuilder.ToString(); + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); + Assert.IsTrue(thumbnail != null); + } + + [TestMethod] + public void CheckThatWidthAndHeightAreParsedCorrectly1() + { + SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); + svgThumbnailProvider.SvgContents = @" + +"; + + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); + Assert.IsTrue(thumbnail != null); + } + + [TestMethod] + public void CheckThatWidthAndHeightAreParsedCorrectly2() + { + SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); + svgThumbnailProvider.SvgContents = @" + + +"; + + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); + Assert.IsTrue(thumbnail != null); + } + + [TestMethod] + public void CheckThatWidthAndHeightAreParsedCorrectly3() + { + SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); + svgThumbnailProvider.SvgContents = @" + + +"; + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); Assert.IsTrue(thumbnail != null); } @@ -57,7 +125,9 @@ namespace SvgThumbnailProviderUnitTests svgBuilder.AppendLine("

foo

"); SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(svgBuilder.ToString(), 256); + svgThumbnailProvider.SvgContents = svgBuilder.ToString(); + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); Assert.IsTrue(thumbnail == null); } @@ -65,7 +135,9 @@ namespace SvgThumbnailProviderUnitTests public void CheckNoSvgEmptyStringShouldReturnNullBitmap() { SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(string.Empty, 256); + svgThumbnailProvider.SvgContents = string.Empty; + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); Assert.IsTrue(thumbnail == null); } @@ -73,7 +145,10 @@ namespace SvgThumbnailProviderUnitTests public void CheckNoSvgNullStringShouldReturnNullBitmap() { SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(null, 256); + svgThumbnailProvider.SvgContents = string.Empty; + svgThumbnailProvider.SvgContentsReady.Set(); + + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); Assert.IsTrue(thumbnail == null); } @@ -82,7 +157,9 @@ namespace SvgThumbnailProviderUnitTests { string content = ""; SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(content, 0); + svgThumbnailProvider.SvgContents = content; + svgThumbnailProvider.SvgContentsReady.Set(); + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(0); Assert.IsTrue(thumbnail == null); } @@ -104,7 +181,10 @@ namespace SvgThumbnailProviderUnitTests svgBuilder.AppendLine(""); SvgThumbnailProvider svgThumbnailProvider = new SvgThumbnailProvider(null); - Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(svgBuilder.ToString(), 256); + svgThumbnailProvider.SvgContents = svgBuilder.ToString(); + svgThumbnailProvider.SvgContentsReady.Set(); + + Bitmap thumbnail = svgThumbnailProvider.GetThumbnail(256); Assert.IsTrue(thumbnail != null); } diff --git a/src/modules/previewpane/common/Utilities/SvgPreviewHandlerHelper.cs b/src/modules/previewpane/common/Utilities/SvgPreviewHandlerHelper.cs index 1c4f7b66de..5cb346d70e 100644 --- a/src/modules/previewpane/common/Utilities/SvgPreviewHandlerHelper.cs +++ b/src/modules/previewpane/common/Utilities/SvgPreviewHandlerHelper.cs @@ -15,6 +15,11 @@ namespace Common.Utilities /// public static class SvgPreviewHandlerHelper { + private const string WidthAttribute = "width=\""; + private const string HeightAttribute = "height=\""; + private const string StyleAttribute = "style=\""; + private const string ViewboxAttribute = "viewBox=\""; + /// /// Dictionary of elements in lower case that are blocked from Svg for preview pane. /// Reference for list of Svg Elements: https://developer.mozilla.org/docs/Web/SVG/Element. @@ -65,6 +70,83 @@ namespace Common.Utilities return foundBlockedElement; } + private static string GetAttributeValue(int attributeNameLength, string data, int startIndex) + { + if (startIndex == -1) + { + return string.Empty; + } + + int start = startIndex + attributeNameLength; + int end = data.IndexOf("\"", start, StringComparison.InvariantCultureIgnoreCase); + return data.Substring(start, end - start); + } + + private static string RemoveAttribute(string data, int startIndex, string attributeName, out int numRemoved) + { + numRemoved = 0; + + if (startIndex == -1) + { + return data; + } + + int end = data.IndexOf("\"", startIndex + attributeName.Length, StringComparison.InvariantCultureIgnoreCase) + 1; + numRemoved = end - startIndex; + return data.Remove(startIndex, numRemoved); + } + + private static string RemoveXmlProlog(string s, int prefixLength, out int numRemoved) + { + numRemoved = 0; + int startIndex = s.IndexOf("", startIndex, StringComparison.InvariantCultureIgnoreCase); + if (endIndex != -1) + { + numRemoved = endIndex + 2 - startIndex; + return s.Remove(startIndex, numRemoved); + } + } + + return s; + } + + private static int FindFirstXmlOpenTagIndex(string s) + { + int index = 0; + + while ((index = s.IndexOf('<', index)) != -1) + { + if (index < s.Length - 1 && s[index + 1] != '?') + { + return index; + } + + index++; + } + + return -1; + } + + private static int FindFirstXmlCloseTagIndex(string s) + { + int index = 1; + + while ((index = s.IndexOf('>', index)) != -1) + { + if (index > 0 && s[index - 1] != '?') + { + return index; + } + + index++; + } + + return -1; + } + /// /// Add proper /// @@ -72,49 +154,57 @@ namespace Common.Utilities /// Returns modified svgData with added style public static string AddStyleSVG(string stringSvgData) { - XElement svgData = XElement.Parse(stringSvgData); - - var attributes = svgData.Attributes(); - string width = string.Empty; - string height = string.Empty; - string widthR = string.Empty; - string heightR = string.Empty; - string oldStyle = string.Empty; - bool hasViewBox = false; - - // Get width and height of element and remove it afterwards because it will be added inside style attribute - for (int i = 0; i < attributes.Count(); i++) + int firstXmlOpenTagIndex = FindFirstXmlOpenTagIndex(stringSvgData); + if (firstXmlOpenTagIndex == -1) { - if (attributes.ElementAt(i).Name == "height") - { - height = attributes.ElementAt(i).Value; - attributes.ElementAt(i).Remove(); - i--; - } - else if (attributes.ElementAt(i).Name == "width") - { - width = attributes.ElementAt(i).Value; - attributes.ElementAt(i).Remove(); - i--; - } - else if (attributes.ElementAt(i).Name == "style") - { - oldStyle = attributes.ElementAt(i).Value; - attributes.ElementAt(i).Remove(); - i--; - } - else if (attributes.ElementAt(i).Name == "viewBox") - { - hasViewBox = true; - } + return stringSvgData; } - svgData.ReplaceAttributes(attributes); + int firstXmlCloseTagIndex = FindFirstXmlCloseTagIndex(stringSvgData); + if (firstXmlCloseTagIndex == -1) + { + return stringSvgData; + } + + stringSvgData = RemoveXmlProlog(stringSvgData, firstXmlOpenTagIndex, out int numRemoved); + + firstXmlOpenTagIndex -= numRemoved; + firstXmlCloseTagIndex -= numRemoved; + + int widthIndex = stringSvgData.IndexOf(WidthAttribute, firstXmlOpenTagIndex, firstXmlCloseTagIndex, StringComparison.InvariantCultureIgnoreCase); + int heightIndex = stringSvgData.IndexOf(HeightAttribute, firstXmlOpenTagIndex, firstXmlCloseTagIndex, StringComparison.InvariantCultureIgnoreCase); + int styleIndex = stringSvgData.IndexOf(StyleAttribute, firstXmlOpenTagIndex, firstXmlCloseTagIndex, StringComparison.InvariantCultureIgnoreCase); + + string width = GetAttributeValue(WidthAttribute.Length, stringSvgData, widthIndex); + string height = GetAttributeValue(HeightAttribute.Length, stringSvgData, heightIndex); + string oldStyle = GetAttributeValue(StyleAttribute.Length, stringSvgData, styleIndex); + + bool hasViewBox = stringSvgData.IndexOf(ViewboxAttribute, firstXmlOpenTagIndex, firstXmlCloseTagIndex - firstXmlOpenTagIndex, StringComparison.InvariantCultureIgnoreCase) != -1; + + stringSvgData = RemoveAttribute(stringSvgData, widthIndex, WidthAttribute, out numRemoved); + if (heightIndex != -1 && heightIndex > widthIndex) + { + heightIndex -= numRemoved; + } + + if (styleIndex != -1 && styleIndex > widthIndex) + { + styleIndex -= numRemoved; + } + + stringSvgData = RemoveAttribute(stringSvgData, heightIndex, HeightAttribute, out numRemoved); + if (styleIndex != -1 && styleIndex > heightIndex) + { + styleIndex -= numRemoved; + } + + stringSvgData = RemoveAttribute(stringSvgData, styleIndex, StyleAttribute, out numRemoved); - height = CheckUnit(height); width = CheckUnit(width); - heightR = RemoveUnit(height); - widthR = RemoveUnit(width); + height = CheckUnit(height); + + string widthR = RemoveUnit(width); + string heightR = RemoveUnit(height); string centering = "position: absolute; top: 50%; left: 50%; transform: translate(-50%, -50%);"; @@ -122,16 +212,19 @@ namespace Common.Utilities string scaling = $"max-width: {width} ; max-height: {height} ;"; scaling += $" _height:expression(this.scrollHeight > {heightR} ? \" {height}\" : \"auto\"); _width:expression(this.scrollWidth > {widthR} ? \"{width}\" : \"auto\");"; - svgData.Add(new XAttribute("style", scaling + centering + oldStyle)); + string newStyle = $"style=\"{scaling}{centering}{oldStyle}\""; + int insertAt = stringSvgData.IndexOf(">", StringComparison.InvariantCultureIgnoreCase); + + stringSvgData = stringSvgData.Insert(insertAt, " " + newStyle); if (!hasViewBox) { // Fixes https://github.com/microsoft/PowerToys/issues/18107 - string viewBox = $"0 0 {widthR} {heightR}"; - svgData.Add(new XAttribute("viewBox", viewBox)); + string viewBox = $"viewBox=\"0 0 {widthR} {heightR}\""; + stringSvgData = stringSvgData.Insert(insertAt, " " + viewBox); } - return svgData.ToString(); + return stringSvgData; } ///