Create a ListMutation abstraction

A possible end goal here is the ImageList type no longer existing.
Still have a bunch to do, including: fixing recovery image ordering,
handling remaining ImageList operations, making deletion cleaner (with
disposal), and maybe doing some more FDesktop refactoring.

A general goal of this refactoring is to make things less fragile (e.g.
selection is based on the items rather than finicky algorithms, change
tracking is based on checking for diffs rather than heuristics, etc.).
This commit is contained in:
Ben Olden-Cooligan 2019-07-19 22:13:54 -04:00
parent 5154afe445
commit cd47c8416d
9 changed files with 315 additions and 307 deletions

View File

@ -0,0 +1,211 @@
using System.Collections.Generic;
using System.Linq;
namespace NAPS2.Images
{
public abstract class ListMutation
{
public abstract void Apply<T>(List<T> list, ListSelection<T> selection);
/// <summary>
/// Whether the mutation won't affect items outside the range of selected indices (both before and after the mutation).
///
/// For example, moving an item up from index 4 to index 5 does not affect any items besides at indices 4 and 5.
/// </summary>
public virtual bool OnlyAffectsSelectionRange => false;
public class MoveDown : ListMutation
{
public override bool OnlyAffectsSelectionRange => true;
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
int lowerBound = 0;
foreach (int i in selection.ToSelectedIndices(list))
{
if (i != lowerBound++)
{
var item = list[i];
list.RemoveAt(i);
list.Insert(i - 1, item);
}
}
}
}
public class MoveUp : ListMutation
{
public override bool OnlyAffectsSelectionRange => true;
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
int upperBound = list.Count - 1;
foreach (int i in selection.ToSelectedIndices(list).Reverse())
{
if (i != upperBound--)
{
var item = list[i];
list.RemoveAt(i);
list.Insert(i + 1, item);
}
}
}
}
public class MoveTo : ListMutation
{
private readonly int destinationIndex;
public MoveTo(int destinationIndex)
{
this.destinationIndex = destinationIndex;
}
public override bool OnlyAffectsSelectionRange => true;
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
var indexList = selection.ToSelectedIndices(list).ToList();
var bottom = indexList.Where(x => x < destinationIndex).OrderByDescending(x => x).ToList();
var top = indexList.Where(x => x >= destinationIndex).OrderBy(x => x).ToList();
int offset = 1;
foreach (int i in bottom)
{
var item = list[i];
list.RemoveAt(i);
list.Insert(destinationIndex - offset, item);
offset++;
}
offset = 0;
foreach (int i in top)
{
var item = list[i];
list.RemoveAt(i);
list.Insert(destinationIndex + offset, item);
offset++;
}
}
}
public class Interleave : ListMutation
{
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
// Partition the image list in two
int count = list.Count;
int split = (count + 1) / 2;
var p1 = list.Take(split).ToList();
var p2 = list.Skip(split).ToList();
// Rebuild the image list, taking alternating images from each the partitions
list.Clear();
for (int i = 0; i < count; ++i)
{
list.Add(i % 2 == 0 ? p1[i / 2] : p2[i / 2]);
}
selection.Clear();
}
}
public class Deinterleave : ListMutation
{
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
// Duplicate the list
int count = list.Count;
int split = (count + 1) / 2;
var copy = list.ToList();
// Rebuild the image list, even-indexed images first
list.Clear();
for (int i = 0; i < split; ++i)
{
list.Add(copy[i * 2]);
}
for (int i = 0; i < (count - split); ++i)
{
list.Add(copy[i * 2 + 1]);
}
selection.Clear();
}
}
public class AltInterleave : ListMutation
{
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
// Partition the image list in two
int count = list.Count;
int split = (count + 1) / 2;
var p1 = list.Take(split).ToList();
var p2 = list.Skip(split).ToList();
// Rebuild the image list, taking alternating images from each the partitions (the latter in reverse order)
list.Clear();
for (int i = 0; i < count; ++i)
{
list.Add(i % 2 == 0 ? p1[i / 2] : p2[p2.Count - 1 - i / 2]);
}
selection.Clear();
}
}
public class AltDeinterleave : ListMutation
{
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
// Duplicate the list
int count = list.Count;
int split = (count + 1) / 2;
var copy = list.ToList();
// Rebuild the image list, even-indexed images first (odd-indexed images in reverse order)
list.Clear();
for (int i = 0; i < split; ++i)
{
list.Add(copy[i * 2]);
}
for (int i = count - split - 1; i >= 0; --i)
{
list.Add(copy[i * 2 + 1]);
}
selection.Clear();
}
}
public class ReverseAll : ListMutation
{
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
list.Reverse();
}
}
public class ReverseSelection : ListMutation
{
public override bool OnlyAffectsSelectionRange => true;
public override void Apply<T>(List<T> list, ListSelection<T> selection)
{
var indexList = selection.ToSelectedIndices(list).ToList();
int pairCount = indexList.Count / 2;
// Swap pairs in the selection, excluding the middle element (if the total count is odd)
for (int i = 0; i < pairCount; i++)
{
int x = indexList[i];
int y = indexList[indexList.Count - i - 1];
(list[x], list[y]) = (list[y], list[x]);
}
}
}
}
}

View File

@ -0,0 +1,35 @@
using System.Collections;
using System.Collections.Generic;
using NAPS2.Util;
namespace NAPS2.Images
{
public static class ListSelection
{
public static ListSelection<T> FromSelectedIndices<T>(List<T> list, IEnumerable<int> selectedIndices)
{
return new ListSelection<T>(list.ElementsAt(selectedIndices));
}
}
public class ListSelection<T> : IEnumerable<T>
{
private readonly HashSet<T> internalSelection;
public ListSelection(IEnumerable<T> selectedItems)
{
internalSelection = new HashSet<T>(selectedItems);
}
public void Clear()
{
internalSelection.Clear();
}
public IEnumerable<int> ToSelectedIndices(List<T> list) => list.IndiciesOf(internalSelection);
public IEnumerator<T> GetEnumerator() => internalSelection.GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}
}

View File

@ -105,6 +105,7 @@ namespace NAPS2.Images
public EventHandler FullyDisposed;
// TODO: Not using this any more, need to find a better way
public void MovedTo(int index)
{
Metadata.Index = index;

View File

@ -27,252 +27,6 @@ namespace NAPS2.Images
public List<ScannedImage> Images { get; }
public IEnumerable<int> MoveUp(IEnumerable<int> selection)
{
lock (this)
{
var newSelection = new int[selection.Count()];
int lowerBound = 0;
int j = 0;
foreach (int i in selection.OrderBy(x => x))
{
if (i != lowerBound++)
{
ScannedImage img = Images[i];
Images.RemoveAt(i);
Images.Insert(i - 1, img);
img.MovedTo(i - 1);
newSelection[j++] = i - 1;
}
else
{
newSelection[j++] = i;
}
}
return newSelection;
}
}
public IEnumerable<int> MoveDown(IEnumerable<int> selection)
{
lock (this)
{
var newSelection = new int[selection.Count()];
int upperBound = Images.Count - 1;
int j = 0;
foreach (int i in selection.OrderByDescending(x => x))
{
if (i != upperBound--)
{
ScannedImage img = Images[i];
Images.RemoveAt(i);
Images.Insert(i + 1, img);
img.MovedTo(i + 1);
newSelection[j++] = i + 1;
}
else
{
newSelection[j++] = i;
}
}
return newSelection;
}
}
public IEnumerable<int> MoveTo(IEnumerable<int> selection, int index)
{
lock (this)
{
var selList = selection.ToList();
var bottom = selList.Where(x => x < index).OrderByDescending(x => x).ToList();
var top = selList.Where(x => x >= index).OrderBy(x => x).ToList();
int offset = 1;
foreach (int i in bottom)
{
ScannedImage img = Images[i];
Images.RemoveAt(i);
Images.Insert(index - offset, img);
img.MovedTo(index - offset);
offset++;
}
offset = 0;
foreach (int i in top)
{
ScannedImage img = Images[i];
Images.RemoveAt(i);
Images.Insert(index + offset, img);
img.MovedTo(index + offset);
offset++;
}
return Enumerable.Range(index - bottom.Count, selList.Count);
}
}
public void Delete(IEnumerable<int> selection)
{
lock (this)
{
// TODO
//using (RecoveryImage.DeferSave())
//{
foreach (ScannedImage img in Images.ElementsAt(selection))
{
img.Dispose();
}
Images.RemoveAll(selection);
//}
}
}
public IEnumerable<int> Interleave(IEnumerable<int> selection)
{
lock (this)
{
// Partition the image list in two
int count = Images.Count;
int split = (count + 1) / 2;
var p1 = Images.Take(split).ToList();
var p2 = Images.Skip(split).ToList();
// Rebuild the image list, taking alternating images from each the partitions
Images.Clear();
for (int i = 0; i < count; ++i)
{
Images.Add(i % 2 == 0 ? p1[i / 2] : p2[i / 2]);
}
// TODO
// RecoveryImage.Refresh(Images);
// Clear the selection (may be changed in the future to maintain it, but not necessary)
return Enumerable.Empty<int>();
}
}
public IEnumerable<int> Deinterleave(IEnumerable<int> selection)
{
lock (this)
{
// Duplicate the list
int count = Images.Count;
int split = (count + 1) / 2;
var images = Images.ToList();
// Rebuild the image list, even-indexed images first
Images.Clear();
for (int i = 0; i < split; ++i)
{
Images.Add(images[i * 2]);
}
for (int i = 0; i < (count - split); ++i)
{
Images.Add(images[i * 2 + 1]);
}
// TODO
// RecoveryImage.Refresh(Images);
// Clear the selection (may be changed in the future to maintain it, but not necessary)
return Enumerable.Empty<int>();
}
}
public IEnumerable<int> AltInterleave(IEnumerable<int> selectedIndices)
{
lock (this)
{
// Partition the image list in two
int count = Images.Count;
int split = (count + 1) / 2;
var p1 = Images.Take(split).ToList();
var p2 = Images.Skip(split).ToList();
// Rebuild the image list, taking alternating images from each the partitions (the latter in reverse order)
Images.Clear();
for (int i = 0; i < count; ++i)
{
Images.Add(i % 2 == 0 ? p1[i / 2] : p2[p2.Count - 1 - i / 2]);
}
// TODO
// RecoveryImage.Refresh(Images);
// Clear the selection (may be changed in the future to maintain it, but not necessary)
return Enumerable.Empty<int>();
}
}
public IEnumerable<int> AltDeinterleave(IEnumerable<int> selectedIndices)
{
lock (this)
{
// Duplicate the list
int count = Images.Count;
int split = (count + 1) / 2;
var images = Images.ToList();
// Rebuild the image list, even-indexed images first (odd-indexed images in reverse order)
Images.Clear();
for (int i = 0; i < split; ++i)
{
Images.Add(images[i * 2]);
}
for (int i = count - split - 1; i >= 0; --i)
{
Images.Add(images[i * 2 + 1]);
}
// TODO
// RecoveryImage.Refresh(Images);
// Clear the selection (may be changed in the future to maintain it, but not necessary)
return Enumerable.Empty<int>();
}
}
public IEnumerable<int> Reverse()
{
lock (this)
{
Reverse(Enumerable.Range(0, Images.Count));
// Selection is unpredictable, so clear it
return Enumerable.Empty<int>();
}
}
public IEnumerable<int> Reverse(IEnumerable<int> selection)
{
lock (this)
{
var selectionList = selection.ToList();
int pairCount = selectionList.Count / 2;
// Swap pairs in the selection, excluding the middle element (if the total count is odd)
for (int i = 0; i < pairCount; i++)
{
int x = selectionList[i];
int y = selectionList[selectionList.Count - i - 1];
var temp = Images[x];
Images[x] = Images[y];
Images[y] = temp;
}
// TODO
// RecoveryImage.Refresh(Images);
// Selection stays the same, so is easy to maintain
return selectionList;
}
}
public async Task RotateFlip(IEnumerable<int> selection, double angle)
{
var images = Images.ElementsAt(selection).ToList();

View File

@ -144,6 +144,8 @@
</Compile>
<Compile Include="Images\HoughLineDeskewer.cs" />
<Compile Include="Images\Deskewer.cs" />
<Compile Include="Images\ListMutation.cs" />
<Compile Include="Images\ListSelection.cs" />
<Compile Include="Images\Storage\NotToBeUsedStorageManager.cs" />
<Compile Include="Remoting\ClientServer\ClientContext.cs" />
<Compile Include="Remoting\ClientServer\ClientContextFactory.cs" />

View File

@ -39,12 +39,22 @@ namespace NAPS2.Util
}
}
/// <summary>
/// Removes multiple elements from the list.
/// </summary>
/// <param name="list"></param>
/// <param name="elements"></param>
public static void RemoveAll<T>(this IList<T> list, IEnumerable<T> elements)
{
list.RemoveAllAt(list.IndiciesOf(elements));
}
/// <summary>
/// Removes multiple elements from the list at the specified indices.
/// </summary>
/// <param name="list"></param>
/// <param name="indices"></param>
public static void RemoveAll(this IList list, IEnumerable<int> indices)
public static void RemoveAllAt<T>(this IList<T> list, IEnumerable<int> indices)
{
int offset = 0;
foreach (int i in indices.OrderBy(x => x))
@ -74,6 +84,7 @@ namespace NAPS2.Util
/// <returns></returns>
public static IEnumerable<int> IndiciesOf<T>(this IList<T> list, IEnumerable<T> elements)
{
// TODO: O(n)
return elements.Select(list.IndexOf);
}

View File

@ -459,8 +459,14 @@ namespace NAPS2.WinForms
Pipes.KillServer();
if (!SkipRecoveryCleanup)
{
imageList.Delete(Enumerable.Range(0, imageList.Images.Count));
imageContext.Dispose();
try
{
imageContext.Dispose();
}
catch (Exception ex)
{
Log.ErrorException("ImageContext.Dispose failed", ex);
}
}
closed = true;
renderThumbnailsWaitHandle.Set();
@ -653,9 +659,9 @@ namespace NAPS2.WinForms
UpdateToolbar();
}
private void UpdateThumbnails(IEnumerable<int> selection, bool scrollToSelection, bool optimizeForSelection)
private void UpdateThumbnails(IEnumerable<int> selection, bool scrollToSelection, bool optimizeForSelectionRange)
{
thumbnailList1.UpdatedImages(imageList.Images, optimizeForSelection ? SelectedIndices.Concat(selection).ToList() : null);
thumbnailList1.UpdatedImages(imageList.Images, optimizeForSelectionRange ? SelectedIndices.Concat(selection).ToList() : null);
SelectedIndices = selection;
UpdateToolbar();
@ -818,7 +824,11 @@ namespace NAPS2.WinForms
{
if (MessageBox.Show(string.Format(MiscResources.ConfirmClearItems, imageList.Images.Count), MiscResources.Clear, MessageBoxButtons.OKCancel, MessageBoxIcon.Question) == DialogResult.OK)
{
imageList.Delete(Enumerable.Range(0, imageList.Images.Count));
foreach (var image in imageList.Images)
{
image.Dispose();
}
imageList.Images.Clear();
DeleteThumbnails();
changeTracker.Clear();
}
@ -831,7 +841,11 @@ namespace NAPS2.WinForms
{
if (MessageBox.Show(string.Format(MiscResources.ConfirmDeleteItems, SelectedIndices.Count()), MiscResources.Delete, MessageBoxButtons.OKCancel, MessageBoxIcon.Question) == DialogResult.OK)
{
imageList.Delete(SelectedIndices);
foreach (var image in imageList.Images.ElementsAt(SelectedIndices))
{
image.Dispose();
}
imageList.Images.RemoveAllAt(SelectedIndices);
DeleteThumbnails();
if (imageList.Images.Any())
{
@ -845,6 +859,18 @@ namespace NAPS2.WinForms
}
}
private void MutateList(ListMutation mutation)
{
var originalList = imageList.Images.ToList();
var selection = ListSelection.FromSelectedIndices(imageList.Images, SelectedIndices);
mutation.Apply(imageList.Images, selection);
UpdateThumbnails(selection.ToSelectedIndices(imageList.Images), true, mutation.OnlyAffectsSelectionRange);
if (!originalList.SequenceEqual(imageList.Images))
{
changeTracker.Made();
}
}
private void SelectAll()
{
SelectedIndices = Enumerable.Range(0, imageList.Images.Count);
@ -852,22 +878,12 @@ namespace NAPS2.WinForms
private void MoveDown()
{
if (!SelectedIndices.Any())
{
return;
}
UpdateThumbnails(imageList.MoveDown(SelectedIndices), true, true);
changeTracker.Made();
MutateList(new ListMutation.MoveDown());
}
private void MoveUp()
{
if (!SelectedIndices.Any())
{
return;
}
UpdateThumbnails(imageList.MoveUp(SelectedIndices), true, true);
changeTracker.Made();
MutateList(new ListMutation.MoveUp());
}
private async Task RotateLeft()
@ -980,7 +996,11 @@ namespace NAPS2.WinForms
{
SafeInvoke(() =>
{
imageList.Delete(imageList.Images.IndiciesOf(images));
foreach (var image in images)
{
image.Dispose();
}
imageList.Images.RemoveAll(images);
DeleteThumbnails();
});
}
@ -993,7 +1013,11 @@ namespace NAPS2.WinForms
{
if (ConfigProvider.Get(c => c.DeleteAfterSaving))
{
imageList.Delete(imageList.Images.IndiciesOf(images));
foreach (var image in images)
{
image.Dispose();
}
imageList.Images.RemoveAll(images);
DeleteThumbnails();
}
}
@ -1547,62 +1571,32 @@ namespace NAPS2.WinForms
private void tsInterleave_Click(object sender, EventArgs e)
{
if (imageList.Images.Count < 3)
{
return;
}
UpdateThumbnails(imageList.Interleave(SelectedIndices), true, false);
changeTracker.Made();
MutateList(new ListMutation.Interleave());
}
private void tsDeinterleave_Click(object sender, EventArgs e)
{
if (imageList.Images.Count < 3)
{
return;
}
UpdateThumbnails(imageList.Deinterleave(SelectedIndices), true, false);
changeTracker.Made();
MutateList(new ListMutation.Deinterleave());
}
private void tsAltInterleave_Click(object sender, EventArgs e)
{
if (imageList.Images.Count < 3)
{
return;
}
UpdateThumbnails(imageList.AltInterleave(SelectedIndices), true, false);
changeTracker.Made();
MutateList(new ListMutation.AltInterleave());
}
private void tsAltDeinterleave_Click(object sender, EventArgs e)
{
if (imageList.Images.Count < 3)
{
return;
}
UpdateThumbnails(imageList.AltDeinterleave(SelectedIndices), true, false);
changeTracker.Made();
MutateList(new ListMutation.AltDeinterleave());
}
private void tsReverseAll_Click(object sender, EventArgs e)
{
if (imageList.Images.Count < 2)
{
return;
}
UpdateThumbnails(imageList.Reverse(), true, false);
changeTracker.Made();
MutateList(new ListMutation.ReverseAll());
}
private void tsReverseSelected_Click(object sender, EventArgs e)
{
if (SelectedIndices.Count() < 2)
{
return;
}
UpdateThumbnails(imageList.Reverse(SelectedIndices), true, true);
changeTracker.Made();
MutateList(new ListMutation.ReverseSelection());
}
#endregion
@ -1883,8 +1877,7 @@ namespace NAPS2.WinForms
int index = GetDragIndex(e);
if (index != -1)
{
UpdateThumbnails(imageList.MoveTo(SelectedIndices, index), true, true);
changeTracker.Made();
MutateList(new ListMutation.MoveTo(index));
}
}

View File

@ -184,7 +184,7 @@ namespace NAPS2.WinForms
{
profileNameTracker.DeletingProfile(profile.DisplayName);
}
profileManager.Profiles.RemoveAll(lvProfiles.SelectedIndices.OfType<int>());
profileManager.Profiles.RemoveAllAt(lvProfiles.SelectedIndices.OfType<int>());
if (profileManager.Profiles.Count == 1)
{
profileManager.DefaultProfile = profileManager.Profiles.First();

View File

@ -481,7 +481,8 @@ namespace NAPS2.WinForms
// Need to dispose the bitmap first to avoid file access issues
tiffViewer1.Image?.Dispose();
// Actually delete the image
ImageList.Delete(Enumerable.Range(ImageIndex, 1));
ImageList.Images[ImageIndex].Dispose();
ImageList.Images.RemoveAt(ImageIndex);
// Update FDesktop in the background
DeleteCallback();