Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions .github/workflows/CI.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ jobs:
name: trx-results
path: |
**/*.trx
**/TestResults/crash-retries.json
if-no-files-found: warn

- uses: actions/upload-artifact@v7
Expand Down
7 changes: 7 additions & 0 deletions Lib/src/unit++/main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@ int main(int argc, const char* argv[])
retval = 1;
}
printf("DEBUG: unit++ main end (retval=%d)\n", retval); fflush(stdout);
#if defined(UNITPP_WINDOWS)
// The OS text-input stack loaded by text shaping never finishes teardown without a
// message pump, so end the process once tests and GlobalTeardown have run. Static
// destructors are skipped.
fflush(NULL);
::TerminateProcess(::GetCurrentProcess(), static_cast<UINT>(retval));
#endif
return retval;
}

Expand Down
63 changes: 57 additions & 6 deletions Src/Common/RenderVerification/RenderSnapshotVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ public static class RenderSnapshotVerifier
{
private const string UpdateBaselinesEnvVar = "FW_UPDATE_RENDER_BASELINES";
private const string FontQualityEnvVar = "FW_FONT_QUALITY";
private const int MaxAllowedPixelDifferences = 4;
// Font-smoothing drift between machines peaks at 50 touched pixels and magnitude
// 1.57, three channels off about 8 levels each; a shifted glyph touches thousands
// at a magnitude near 1.
private const int MaxAllowedPixelDifferences = 100;
private const double MaxAllowedDifferenceMagnitude = 10.0;
private const string DeterministicRenderFontFamily = "Segoe UI";
private const int DpiAwarenessInvalid = -1;
private const int DpiAwarenessUnaware = 0;
Expand Down Expand Up @@ -81,7 +85,11 @@ public static RenderBaselineVerificationResult Verify(Bitmap actualBitmap, strin
{
var savedArtifact = LoadSavedArtifact(expectedBitmap, verifiedPath, verifiedMetadataPath);
var diffSummary = CompareBitmaps(expectedBitmap, actualBitmap);
if (diffSummary.DifferentPixelCount <= MaxAllowedPixelDifferences)
// A size change is a layout regression, so it fails whatever the pixel tolerance

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think "it fails regardless of the pixel tolerance" (as you have below) is a better wording

// allows.
bool sizeMatches = expectedBitmap.Width == actualBitmap.Width
&& expectedBitmap.Height == actualBitmap.Height;
if (sizeMatches && IsWithinTolerance(diffSummary))
{
DeleteIfPresent(receivedPath);
DeleteIfPresent(receivedMetadataPath);
Expand Down Expand Up @@ -113,6 +121,7 @@ public static RenderBaselineVerificationResult Verify(Bitmap actualBitmap, strin
ScenarioId = scenarioId,
SnapshotName = name,
AllowedDifferentPixelCount = MaxAllowedPixelDifferences,
AllowedDifferenceMagnitude = MaxAllowedDifferenceMagnitude,
SavedBaseline = savedArtifact,
CurrentRun = currentArtifact,
Diff = diffSummary,
Expand Down Expand Up @@ -299,11 +308,18 @@ private static string BuildFailureMessage(
{
var builder = new StringBuilder();
builder.AppendFormat(CultureInfo.InvariantCulture,
"Render output for '{0}' differed from baseline by {1} pixels; {2} or fewer differences are allowed.",
"Render output for '{0}' differed from baseline by {1} pixels ({2} full-pixel equivalents); fewer than {3} pixels and fewer than {4} full-pixel equivalents are allowed.",
scenarioId,
report.Diff.DifferentPixelCount,
report.AllowedDifferentPixelCount);
FormatMagnitude(report.Diff.DifferenceMagnitude),
report.AllowedDifferentPixelCount,
FormatMagnitude(report.AllowedDifferenceMagnitude));
builder.AppendLine();
if (report.SavedBaseline.ImageWidth != report.CurrentRun.ImageWidth
|| report.SavedBaseline.ImageHeight != report.CurrentRun.ImageHeight)
{
builder.AppendLine("Image size changed, which fails regardless of the pixel tolerance.");
}
builder.AppendLine(FormatArtifactLine("Saved baseline", report.SavedBaseline));
builder.AppendLine(FormatArtifactLine("Current run", report.CurrentRun));
builder.AppendFormat(CultureInfo.InvariantCulture,
Expand Down Expand Up @@ -530,6 +546,35 @@ private static void DeleteIfPresent(string path)
File.Delete(path);
}

/// <summary>
/// Answers whether a diff is small enough to be environment drift rather than a render
/// change. The touched-pixel count and the summed magnitude must both stay under their
/// limit.
/// </summary>
private static bool IsWithinTolerance(RenderPixelDiffSummary summary)
{
return summary.DifferentPixelCount < MaxAllowedPixelDifferences
&& summary.DifferenceMagnitude < MaxAllowedDifferenceMagnitude;
}

/// <summary>
/// Scores one differing pixel. All three channels fully inverted score 1, so a single
/// saturated channel scores a third and the magnitude limit admits three times as many
/// single-channel changes as whole-pixel ones. Alpha is not scored.
/// </summary>
private static double PixelDifferenceMagnitude(Color expected, Color actual)
{
int channelDelta = Math.Abs(expected.R - actual.R)
+ Math.Abs(expected.G - actual.G)
+ Math.Abs(expected.B - actual.B);
return channelDelta / (3.0 * 255.0);
}

private static string FormatMagnitude(double magnitude)
{
return magnitude.ToString("0.##", CultureInfo.InvariantCulture);
}

private static RenderPixelDiffSummary CompareBitmaps(Bitmap expectedBitmap, Bitmap actualBitmap)
{
int maxWidth = Math.Max(expectedBitmap.Width, actualBitmap.Width);
Expand All @@ -546,6 +591,7 @@ private static RenderPixelDiffSummary CompareBitmaps(Bitmap expectedBitmap, Bitm
if (!expectedInBounds || !actualInBounds)
{
summary.DifferentPixelCount++;
summary.DifferenceMagnitude += 1.0;
if (expectedInBounds)
summary.ExpectedOnlyPixelDifferences++;
else if (actualInBounds)
Expand All @@ -554,11 +600,14 @@ private static RenderPixelDiffSummary CompareBitmaps(Bitmap expectedBitmap, Bitm
continue;
}

if (expectedBitmap.GetPixel(x, y) == actualBitmap.GetPixel(x, y))
Color expectedPixel = expectedBitmap.GetPixel(x, y);
Color actualPixel = actualBitmap.GetPixel(x, y);
if (expectedPixel == actualPixel)
continue;

summary.DifferentPixelCount++;
summary.InBoundsPixelDifferences++;
summary.DifferenceMagnitude += PixelDifferenceMagnitude(expectedPixel, actualPixel);
UpdateDiffBounds(summary, x, y);
}
}
Expand Down Expand Up @@ -670,6 +719,7 @@ public sealed class RenderSnapshotComparisonReport
public string ScenarioId { get; set; }
public string SnapshotName { get; set; }
public int AllowedDifferentPixelCount { get; set; }
public double AllowedDifferenceMagnitude { get; set; }
public RenderSnapshotArtifact SavedBaseline { get; set; }
public RenderSnapshotArtifact CurrentRun { get; set; }
public RenderPixelDiffSummary Diff { get; set; }
Expand All @@ -679,6 +729,7 @@ public sealed class RenderSnapshotComparisonReport
public sealed class RenderPixelDiffSummary
{
public int DifferentPixelCount { get; set; }
public double DifferenceMagnitude { get; set; }
public int InBoundsPixelDifferences { get; set; }
public int ExpectedOnlyPixelDifferences { get; set; }
public int ActualOnlyPixelDifferences { get; set; }
Expand All @@ -689,4 +740,4 @@ public sealed class RenderPixelDiffSummary
public int DiffRegionWidth { get; set; }
public int DiffRegionHeight { get; set; }
}
}
}
7 changes: 2 additions & 5 deletions Src/Common/RootSite/RootSiteTests/RenderVerifyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using NUnit.Framework;
using SIL.FieldWorks.Common.RootSites.RenderBenchmark;

Expand All @@ -16,7 +15,7 @@ namespace SIL.FieldWorks.Common.RootSites
/// Each run saves a .received.png and compares it against the committed
/// .verified.png baseline by decoded pixel values, not by the PNG file bytes.
/// Small encoder-level differences are therefore ignored as long as the rendered
/// image differs by fewer than five pixels.
/// image stays within the verifier's pixel-count and difference-magnitude tolerance.
///
/// Each scenario is set up inside its own UndoableUnitOfWork, matching the pattern
/// used by RenderTimingSuiteTests.
Expand All @@ -40,13 +39,11 @@ protected override void CreateTestData()
/// compares decoded pixels against the committed .verified.png baseline.
/// </summary>
[Test, TestCaseSource(nameof(GetVerifyScenarios))]
public async Task VerifyScenario(string scenarioId)
public void VerifyScenario(string scenarioId)
{
var execution = ExecuteScenarioAndCapture(scenarioId, includeWarmRender: false);
if (!execution.Verification.Passed)
Assert.Fail(execution.Verification.FailureMessage);

await Task.CompletedTask;
}

/// <summary>
Expand Down
Loading
Loading