Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Simplify implementation for ImageTagSet equality #1116

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Simplify implementation for ImageTagSet equality #1116

Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
3550ded
Simplify implementation for ImageTagSet equality
Therzok Feb 20, 2023
25cb1aa
Reuse the same tag separator array
Therzok Feb 20, 2023
30ee113
Add a lookup table for most common tags used in images
Therzok Feb 20, 2023
1557d48
Remove unused tags string and improvements to caching
Therzok Feb 21, 2023
455d72b
Forgot to initialize
Therzok Feb 21, 2023
3dd2e28
Correct formatting of tags string
Therzok Feb 21, 2023
cb9c628
Avoid allocating substrings
Therzok Feb 21, 2023
7511a91
Reuse a cached array for the manifest resource names
Therzok Feb 21, 2023
681e4cd
More caching in enumerating alternative files
Therzok Feb 21, 2023
be8d81a
Use specialized APIs to get the handles
Therzok Feb 21, 2023
b4ceca4
This shoudl be static
Therzok Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 118 additions & 28 deletions Xwt/Xwt.Drawing/Image.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
using System.Reflection;
using System.IO;
using System.Collections.Generic;
using System.Diagnostics;

namespace Xwt.Drawing
{
Expand Down Expand Up @@ -252,7 +253,7 @@ static bool ParseImageHints (string baseName, string fileName, string ext, out i
tags = ImageTagSet.Empty;
var firstDelimiter = fileName.IndexOfAny (tagDelimiters);

if (firstDelimiter <= 0 || fileName.Length <= baseName.Length + 1 || !fileName.Substring(0, firstDelimiter).Equals(baseName, StringComparison.Ordinal))
if (firstDelimiter <= 0 || fileName.Length <= baseName.Length + 1 || string.Compare(fileName, 0, baseName, 0, firstDelimiter) != 0)
return false;

fileName = fileName.Substring (0, fileName.Length - ext.Length);
Expand All @@ -268,7 +269,7 @@ static bool ParseImageHints (string baseName, string fileName, string ext, out i
return false;
} else
i2 = fileName.Length;
tags = new ImageTagSet (fileName.Substring (i, i2 - i));
tags = ImageTagSet.Parse (fileName.Substring (i, i2 - i));
return true;
}
else {
Expand All @@ -288,7 +289,7 @@ static bool ParseImageHints (string baseName, string fileName, string ext, out i
return false;
}
if (i2 + 2 < fileName.Length)
tags = new ImageTagSet (fileName.Substring (i2 + 2));
tags = ImageTagSet.Parse (fileName.Substring (i2 + 2));
return true;
}
}
Expand All @@ -307,18 +308,23 @@ public static Image CreateMultiSizeIcon (IEnumerable<Image> images)
// If one of the images is themed, then the whole resulting image will be themed.
// To create the new image, we group images with the same theme but different size, and we create a multi-size icon for those.
// The resulting image is the combination of those multi-size icons.
var allThemes = allImages.OfType<ThemedImage> ().SelectMany (i => i.Images).Select (i => new ImageTagSet (i.Item2)).Distinct ().ToArray ();
var allThemes = allImages
.OfType<ThemedImage> ()
.SelectMany (i => i.Images)
.Select(i => i.Item2)
.Distinct (TagSetEqualityComparer.Instance)
.ToArray ();
List<Tuple<Image, string []>> newImages = new List<Tuple<Image, string []>> ();
foreach (var ts in allThemes) {
List<Image> multiSizeImages = new List<Image> ();
foreach (var i in allImages) {
if (i is ThemedImage)
multiSizeImages.Add (((ThemedImage)i).GetImage (ts.AsArray));
multiSizeImages.Add (((ThemedImage)i).GetImage (ts));
else
multiSizeImages.Add (i);
}
var img = CreateMultiSizeIcon (multiSizeImages);
newImages.Add (new Tuple<Image, string []> (img, ts.AsArray));
newImages.Add (new Tuple<Image, string[]> (img, ts));
}
return new ThemedImage (newImages);
} else {
Expand Down Expand Up @@ -967,36 +973,123 @@ public void ReleaseReference (bool disposing)
public NativeImageRef NextRef { get; set; }
}

class ImageTagSet
sealed class ImageTagCache
{
/* Some stats from an app using Xwt:
1474 dark
1304 contrast
1296 contrast~dark
846 sel
846 dark~sel
132 disabled
120 dark~disabled
116 contrast~disabled
116 contrast~dark~disabled
22 error
22 dark~error
22 contrast~error
22 contrast~dark~error
14 contrast~dark~sel
12 contrast~sel
6 disabled~dark
6 dark~contrast
2 sel~error
2 pressed~dark
2 pressed
2 hover~dark
2 hover
2 dark~sel~error
2 contrast~sel~error
2 active~sel
2 active~dark~sel
2 active~dark
2 active~contrast~dark
2 active~contrast
2 active

Keep in sync with knownTagArrays.
These tag items amount for 97% of the image tags found in images.
*/
readonly string[] knownTags = new[] {
"dark",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why I made the cache this way. Looking back, I think that we can store the original tags string and cache the ImageTagSet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, we can have some presets, but cache tags that are added later as well.

"contrast",
"contrast~dark",
"sel",
"dark~sel",
"disabled",
"dark~disabled",
"contrast~disabled",
"contrast~dark~disabled",
};

readonly ImageTagSet[] knownTagArrays = new[] {
new ImageTagSet(new[] { "dark", }),
new ImageTagSet(new[] { "contrast", }),
new ImageTagSet(new[] { "contrast", "dark", }),
new ImageTagSet(new[] { "sel", }),
new ImageTagSet(new[] { "dark", "sel", }),
new ImageTagSet(new[] { "disabled", }),
new ImageTagSet(new[] { "dark", "disabled", }),
new ImageTagSet(new[] { "contrast", "disabled", }),
new ImageTagSet(new[] { "contrast", "dark", "disabled", }),
};

public ImageTagSet TryGetTagSet(string tags)
{
var index = Array.IndexOf(knownTags, tags);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to a list, and cache any tagset that isn't in the list already.

return index >= 0 ? knownTagArrays[index] : null;
}
}

// As much as I don't like the duplication, it's simpler than accessing a static instance every time.
class TagSetEqualityComparer : IEqualityComparer<string[]>
{
public static TagSetEqualityComparer Instance { get; } = new TagSetEqualityComparer();

public bool Equals(string[] x, string[] y) => x.SequenceEqual(y);

public int GetHashCode(string[] obj)
{
unchecked
{
int c = 0;
foreach (var s in obj)
c %= s.GetHashCode();
return c;
}
}
}

[DebuggerDisplay("{DebuggerDisplay,nq}")]
sealed class ImageTagSet
{
string tags;
string[] tagsArray;

public static readonly ImageTagSet Empty = new ImageTagSet (new string[0]);
static readonly ImageTagCache imageTagCache = new ImageTagCache();
static readonly char[] tagSeparators = { '~' };

public ImageTagSet (string [] tagsArray)
public static ImageTagSet Parse(string tags)
{
this.tagsArray = tagsArray;
Array.Sort (tagsArray);
return imageTagCache.TryGetTagSet(tags) ?? Create(tags);
}

public bool IsEmpty {
get {
return tagsArray.Length == 0;
}
static ImageTagSet Create(string tags)
{
var tagArray = tags.Split(tagSeparators, StringSplitOptions.RemoveEmptyEntries);
Array.Sort(tagArray);

return new ImageTagSet(tagArray);
}

public ImageTagSet (string tags)
public ImageTagSet (string [] tagsArray)
{
tagsArray = tags.Split (new [] { '~' }, StringSplitOptions.RemoveEmptyEntries);
Array.Sort (AsArray);
this.tagsArray = tagsArray;
}

public string AsString {
public bool IsEmpty {
get {
if (tags == null)
tags = string.Join ("~", tagsArray);
return tags;
return tagsArray.Length == 0;
}
}

Expand All @@ -1009,12 +1102,7 @@ public ImageTagSet (string tags)
public override bool Equals (object obj)
{
var other = obj as ImageTagSet;
if (other == null || tagsArray.Length != other.tagsArray.Length)
return false;
for (int n = 0; n < tagsArray.Length; n++)
if (tagsArray [n] != other.tagsArray [n])
return false;
return true;
return other != null && tagsArray.SequenceEqual(other.tagsArray);
}

public override int GetHashCode ()
Expand All @@ -1026,6 +1114,8 @@ public override int GetHashCode ()
return c;
}
}

string DebuggerDisplay => string.Join("~", tagsArray);
}

abstract class ImageLoader
Expand Down