Skip to content
This repository has been archived by the owner on Sep 4, 2024. It is now read-only.

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

Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions Xwt.XamMac/Xwt.Mac/ImageHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ namespace Xwt.Mac
{
public class ImageHandler: ImageBackendHandler
{
static readonly IntPtr sel_alloc = new Selector ("alloc").Handle;
static readonly IntPtr sel_release = new Selector ("release").Handle;
static readonly IntPtr sel_initWithIconRef = new Selector ("initWithIconRef:").Handle;
static readonly IntPtr cls_NSImage = new Class (typeof (NSImage)).Handle;
static readonly IntPtr sel_alloc = Selector.GetHandle ("alloc");
static readonly IntPtr sel_release = Selector.GetHandle ("release");
static readonly IntPtr sel_initWithIconRef = Selector.GetHandle ("initWithIconRef:");
static readonly IntPtr cls_NSImage = Class.GetHandle(typeof (NSImage));

static Dictionary<string, NSImage> stockIcons = new Dictionary<string, NSImage> ();

Expand Down
178 changes: 139 additions & 39 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,8 @@
using System.Reflection;
using System.IO;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;

namespace Xwt.Drawing
{
Expand All @@ -42,6 +44,7 @@ public class Image: XwtObject, IDisposable
internal StyleSet styles;

internal static int[] SupportedScales = { 2 };
internal static string[] SupportedScalesTags = SupportedScales.Select(scale => "@" + scale + "x").ToArray();

internal Image ()
{
Expand Down Expand Up @@ -252,7 +255,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 +271,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 +291,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 +310,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 +975,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 +1104,7 @@ public string [] AsArray {
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 +1116,8 @@ public override int GetHashCode ()
return c;
}
}

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

abstract class ImageLoader
Expand Down Expand Up @@ -1054,9 +1146,11 @@ public override object LoadImage (string fileName)
return img;
}

static ConditionalWeakTable<Assembly, string[]> resourceNamesCache = new ConditionalWeakTable<Assembly, string[]>();
public override IEnumerable<string> GetAlternativeFiles (string fileName, string baseName, string ext)
{
return assembly.GetManifestResourceNames ().Where (f =>
var resourceNames = resourceNamesCache.GetValue(assembly, asm => asm.GetManifestResourceNames());
return resourceNames.Where (f =>
f.StartsWith (baseName, StringComparison.Ordinal) &&
f.EndsWith (ext, StringComparison.Ordinal));
}
Expand Down Expand Up @@ -1090,16 +1184,22 @@ public override object LoadImage (string fileName)

public override IEnumerable<string> GetAlternativeFiles (string fileName, string baseName, string ext)
{
if (!Context.RegisteredStyles.Any ()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check breaks my brain. I don't really understand why it is !Context.RegisteredStyles.Any()

foreach (var s in Image.SupportedScales) {
var fn = baseName + "@" + s + "x" + ext;
if (File.Exists (fn))
yield return fn;
}
} else {
var files = Directory.EnumerateFiles (Path.GetDirectoryName (fileName), Path.GetFileName (baseName) + "*" + ext);
foreach (var f in files)
yield return f;
if (!Context.RegisteredStyles.Any())
{
return EnumerateFilesForRegisteredStyles(baseName, ext);
}
else
{
return Directory.EnumerateFiles(Path.GetDirectoryName(fileName), Path.GetFileName(baseName) + "*" + ext);
}
}

IEnumerable<string> EnumerateFilesForRegisteredStyles (string baseName, string ext)
{
foreach (var scaleTag in Image.SupportedScalesTags) {
var fn = baseName + scaleTag + ext;
if (File.Exists (fn))
yield return fn;
}
}

Expand Down