diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs b/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs index 08944e51c1..fecc2164c2 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/DefaultFontsControl.cs @@ -250,9 +250,8 @@ protected void SetSelectedFonts() bool isGraphiteFont = m_defaultFontFeaturesButton.IsGraphiteFont; m_graphiteGroupBox.Enabled = isGraphiteFont || m_defaultFontFeaturesButton.HasFontFeatures; m_enableGraphiteCheckBox.Enabled = isGraphiteFont; - if (!isGraphiteFont) - m_ws.IsGraphiteEnabled = false; - m_enableGraphiteCheckBox.Checked = m_ws.IsGraphiteEnabled; + // Display only; writing the flag here would mark an untouched writing system changed. + m_enableGraphiteCheckBox.Checked = isGraphiteFont && m_ws.IsGraphiteEnabled; } /// diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs index 7327c881b0..393b23bf0d 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontAttributes.cs @@ -105,6 +105,18 @@ public string FontName set { m_btnFontFeatures.FontName = value; } } + /// ------------------------------------------------------------------------------------ + /// + /// Gets or sets whether the Font Features button offers Graphite features ahead of + /// OpenType features when the current font supports both. + /// + /// ------------------------------------------------------------------------------------ + public bool UseGraphiteFeatures + { + get { CheckDisposed(); return m_btnFontFeatures.UseGraphiteFeatures; } + set { CheckDisposed(); m_btnFontFeatures.UseGraphiteFeatures = value; } + } + /// ------------------------------------------------------------------------------------ /// /// Gets or sets a value indicating whether the controls for super/subscript are enabled or not. diff --git a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs index c50498e8b3..eeaf8ebab4 100644 --- a/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs +++ b/Src/FwCoreDlgs/FwCoreDlgControls/FwFontTab.cs @@ -45,6 +45,7 @@ public partial class FwFontTab : UserControl, IStylesTab private bool m_fIgnoreWsSelectedIndexChange = false; private StyleInfo m_currentStyleInfo; private int m_currentWs = -1; + private ILgWritingSystemFactory m_wsf; private bool m_fFontListIncludesRealNames = false; @@ -81,7 +82,12 @@ public void CheckDisposed() /// ------------------------------------------------------------------------------------ public ILgWritingSystemFactory WritingSystemFactory { - set { CheckDisposed(); m_FontAttributes.WritingSystemFactory = value; } + set + { + CheckDisposed(); + m_wsf = value; + m_FontAttributes.WritingSystemFactory = value; + } } #endregion @@ -302,6 +308,8 @@ public void UpdateForStyle(StyleInfo styleInfo, int ws) // only include the magic font names, not the real ones. FillFontNames(ws > -1); + m_FontAttributes.UseGraphiteFeatures = IsGraphiteEnabled(ws); + m_FontAttributes.ShowingInheritedProperties = true; // Always allow re-setting to unspecified for font attributes // Initialize controls based on whether or not this style inherits from another style. @@ -418,6 +426,18 @@ internal ComboBox FontNamesComboBox } #region private methods + /// + /// Returns whether Graphite rendering is enabled for the given writing system. The + /// default row (-1) covers every writing system, so it reports false. + /// + private bool IsGraphiteEnabled(int ws) + { + if (ws <= 0 || m_wsf == null) + return false; + var writingSystem = m_wsf.get_EngineOrNull(ws); + return writingSystem != null && writingSystem.IsGraphiteEnabled; + } + /// ------------------------------------------------------------------------------------ /// /// Fills the font names. diff --git a/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs b/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs index 7ca12f3f21..85cdd0a937 100644 --- a/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs +++ b/Src/FwCoreDlgs/FwCoreDlgsTests/FwWritingSystemSetupModelTests.cs @@ -760,6 +760,160 @@ public void Model_WritingSystemChanged_NotCalledOnIrrelevantChange() Assert.That(writingSystemChanged, Is.False, "WritingSystemUpdated should not have been called after this change"); } + [Test] + public void Model_WritingSystemChanged_CalledOnDefaultFontFeaturesChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + // Changing only the font features must refresh views that cache rendered output. + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "cv43=1,smcp=1" }; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnInPlaceFontFeaturesChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + // The font dialog edits the existing definition rather than replacing it. + testModel.CurrentDefaultFont.Features = "cv43=1"; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnGraphiteToggle() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.IsGraphiteEnabled = false; + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.IsGraphiteEnabled = true; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnRightToLeftChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.CurrentWsSetupModel.CurrentRightToLeftScript = true; + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_CalledOnNumberingSystemChange() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.CurrentWsSetupModel.CurrentNumberingSystemDefinition = NumberingSystemDefinition.CreateCustomSystem("abcdefghij"); + testModel.Save(); + Assert.That(writingSystemChanged, Is.True, "WritingSystemUpdated should have been called after this change"); + } + + [Test] + public void Model_WritingSystemChanged_NotCalledOnReorderedFontFeatures() + { + var writingSystemChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "smcp=1,cv43=1" }; + testModel.Save(); + testModel.WritingSystemUpdated += (sender, args) => + { + writingSystemChanged = true; + }; + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "cv43=1,smcp=1" }; + testModel.Save(); + Assert.That(writingSystemChanged, Is.False, "Reordering the same features is not a rendering change"); + } + + [Test] + public void Model_WritingSystemChanged_NotCalledWhenListAlsoChanged() + { + var writingSystemChanged = false; + var listChanged = false; + var mockWsManager = new Mock(); + + var container = new TestWSContainer(new[] { "fr", "en" }); + var testModel = new FwWritingSystemSetupModel(container, + FwWritingSystemSetupModel.ListType.Vernacular, mockWsManager.Object); + testModel.WritingSystemUpdated += (sender, args) => { writingSystemChanged = true; }; + testModel.WritingSystemListUpdated += (sender, args) => { listChanged = true; }; + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL"); + testModel.MoveDown(); + testModel.Save(); + Assert.That(listChanged, Is.True, "WritingSystemListUpdated should have been called after this change"); + Assert.That(writingSystemChanged, Is.False, "One refresh is enough when the list update already refreshes"); + } + + [Test] + public void Model_WritingSystemChanged_RaisedAfterUnitOfWorkCloses() + { + Cache.ActionHandlerAccessor.EndUndoTask(); + var testModel = new FwWritingSystemSetupModel(Cache.LangProject, FwWritingSystemSetupModel.ListType.Vernacular, + Cache.ServiceLocator.WritingSystemManager, Cache); + var depthWhenRaised = -1; + testModel.WritingSystemUpdated += (sender, args) => + { + depthWhenRaised = Cache.ActionHandlerAccessor.CurrentDepth; + }; + testModel.CurrentDefaultFont = new FontDefinition("Charis SIL") { Features = "cv43=1,smcp=1" }; + testModel.Save(); + Assert.That(depthWhenRaised, Is.EqualTo(0), "Listeners refresh views, which must not run inside the save's unit of work"); + } + [TestCase(FwWritingSystemSetupModel.ListType.Vernacular)] [TestCase(FwWritingSystemSetupModel.ListType.Analysis)] public void WritingSystemTitle_ChangesByType(FwWritingSystemSetupModel.ListType type) diff --git a/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs b/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs index 52ad084022..a710d61ab9 100644 --- a/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs +++ b/Src/FwCoreDlgs/FwWritingSystemSetupModel.cs @@ -80,7 +80,8 @@ public List WorkingList public event EventHandler WritingSystemListUpdated; /// - /// This event is fired only when the id or abbreviation for a writing system changes + /// Raised once after a save when an existing writing system's id, abbreviation, or + /// rendering settings changed. Not raised when WritingSystemListUpdated is. /// public event EventHandler WritingSystemUpdated; @@ -663,6 +664,9 @@ public void Save() { uowHelper = new NonUndoableUnitOfWorkHelper(Cache.ActionHandlerAccessor); } + // Listeners refresh every window, so notify once, after the unit of work has closed. + var needsRefresh = false; + var listChanged = false; try { @@ -715,6 +719,7 @@ public void Save() else if (workingWs.IsChanged) { var didAbbrevOrIdChange = origWs.Abbreviation != workingWs.Abbreviation; + var didRenderingChange = RenderingChanged(origWs, workingWs); var oldId = origWs.Id; var oldHandle = origWs.Handle; // copy the working writing system content into the original writing system @@ -729,9 +734,9 @@ public void Save() } didAbbrevOrIdChange = true; } - if (didAbbrevOrIdChange) + if (didAbbrevOrIdChange || didRenderingChange) { - WritingSystemUpdated?.Invoke(this, EventArgs.Empty); + needsRefresh = true; } } @@ -767,13 +772,35 @@ public void Save() } finally { - if (CurrentWsListChanged) + listChanged = CurrentWsListChanged; + if (uowHelper != null) + uowHelper.Dispose(); + if (listChanged) { WritingSystemListUpdated?.Invoke(this, EventArgs.Empty); } - if (uowHelper != null) - uowHelper.Dispose(); } + if (needsRefresh && !listChanged) + { + WritingSystemUpdated?.Invoke(this, EventArgs.Empty); + } + } + + /// + /// Returns whether the two definitions would render text differently: another default + /// font, other font features once normalized, or a different Graphite setting, text + /// direction, or numbering system. + /// + private static bool RenderingChanged(CoreWritingSystemDefinition origWs, CoreWritingSystemDefinition workingWs) + { + if (origWs.DefaultFontName != workingWs.DefaultFontName) + return true; + if (FontFeatureSettings.NormalizePreservingLegacy(origWs.DefaultFontFeatures) + != FontFeatureSettings.NormalizePreservingLegacy(workingWs.DefaultFontFeatures)) + return true; + return origWs.IsGraphiteEnabled != workingWs.IsGraphiteEnabled + || origWs.RightToLeftScript != workingWs.RightToLeftScript + || !Equals(origWs.NumberingSystem, workingWs.NumberingSystem); } private static void AddOrMoveInList(ICollection allWritingSystems, int desiredIndex, CoreWritingSystemDefinition workingWs)