svx/inc/CommonStylePreviewRenderer.hxx | 9 - svx/source/dialog/fntctrl.cxx | 119 +++++++++-------------- svx/source/styles/CommonStylePreviewRenderer.cxx | 58 ++++------- svx/source/tbxctrls/tbcontrl.cxx | 79 ++++++--------- 4 files changed, 105 insertions(+), 160 deletions(-)
New commits: commit 3e7e5dad9d8442a74a5d51dddca655b6073d0ff2 Author: Khaled Hosny <kha...@aliftype.com> AuthorDate: Fri Dec 16 00:10:34 2022 +0200 Commit: خالد حسني <kha...@aliftype.com> CommitDate: Fri Dec 16 18:38:53 2022 +0000 tdf#152533: Improve script handling in font preview Use Edit Engine to get the script types instead of trying to duplicate (poorly) its behaviour. This has the advantage of handling weak characters and digits properly and is closer to what would happen in the document itself. We probably should go further and render the preview entirely using Edit Engine (since the current code fails short of proper bidi handling), but this is a bigger change. Change-Id: I975cb2d96a4a18dbd8110686ca09649cab0ec2f1 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144284 Tested-by: Jenkins Reviewed-by: خالد حسني <kha...@aliftype.com> (cherry picked from commit 718af940435ae9d2ac90374e5880ecb38e96252c) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144272 diff --git a/svx/source/dialog/fntctrl.cxx b/svx/source/dialog/fntctrl.cxx index 393fc4e68103..0712bbe92496 100644 --- a/svx/source/dialog/fntctrl.cxx +++ b/svx/source/dialog/fntctrl.cxx @@ -23,10 +23,6 @@ #include <vcl/metric.hxx> #include <vcl/svapp.hxx> #include <vcl/settings.hxx> -#include <unicode/uchar.h> -#include <com/sun/star/uno/Reference.h> -#include <com/sun/star/i18n/BreakIterator.hpp> -#include <comphelper/processfactory.hxx> #include <com/sun/star/i18n/ScriptType.hpp> @@ -46,6 +42,7 @@ #include <svl/cjkoptions.hxx> #include <svl/ctloptions.hxx> +#include <editeng/editeng.hxx> #include <editeng/colritem.hxx> #include <editeng/fontitem.hxx> #include <editeng/editids.hrc> @@ -72,8 +69,6 @@ using namespace ::com::sun::star::uno; using namespace ::com::sun::star::lang; -using ::com::sun::star::i18n::XBreakIterator; -using ::com::sun::star::i18n::BreakIterator; // small helper functions to set fonts @@ -127,6 +122,19 @@ OUString removeCRLF(const OUString& rText) return rText.replace(0xa, ' ').replace(0xd, ' ').trim(); } +struct ScriptInfo +{ + tools::Long textWidth; + SvtScriptType scriptType; + sal_Int32 changePos; + ScriptInfo(SvtScriptType scrptType, sal_Int32 position) + : textWidth(0) + , scriptType(scrptType) + , changePos(position) + { + } +}; + } // end anonymous namespace class FontPrevWin_Impl @@ -137,10 +145,7 @@ class FontPrevWin_Impl VclPtr<Printer> mpPrinter; bool mbDelPrinter; - Reference <XBreakIterator> mxBreak; - std::vector<tools::Long> maTextWidth; - std::deque<sal_Int32> maScriptChg; - std::vector<sal_uInt16> maScriptType; + std::vector<ScriptInfo> maScriptChanges; SvxFont maCJKFont; SvxFont maCTLFont; OUString maText; @@ -236,45 +241,18 @@ void FontPrevWin_Impl::CheckScript() } maScriptText = maText; + maScriptChanges.clear(); - maScriptChg.clear(); - maScriptType.clear(); - maTextWidth.clear(); + auto aEditEngine = EditEngine(nullptr); + aEditEngine.SetText(maScriptText); - if (!mxBreak.is()) + auto aScript = aEditEngine.GetScriptType({ 0, 0, 0, 0 }); + for (sal_Int32 i = 1; i <= maScriptText.getLength(); i++) { - Reference<XComponentContext> xContext = ::comphelper::getProcessComponentContext(); - mxBreak = BreakIterator::create(xContext); - } - - sal_uInt16 nScript = 0; - sal_Int32 nChg = 0; - - while (nChg < maText.getLength()) - { - nScript = mxBreak->getScriptType(maText, nChg); - nChg = mxBreak->endOfScript(maText, nChg, nScript); - if (nChg < maText.getLength() && nChg > 0 && - (css::i18n::ScriptType::WEAK == - mxBreak->getScriptType(maText, nChg - 1))) - { - int8_t nType = u_charType(maText[nChg]); - if (nType == U_NON_SPACING_MARK || nType == U_ENCLOSING_MARK || - nType == U_COMBINING_SPACING_MARK) - { - maScriptChg.push_back(nChg - 1); - } - else - { - maScriptChg.push_back(nChg); - } - } - else - { - maScriptChg.push_back(nChg); - } - maScriptType.push_back(nScript); - maTextWidth.push_back(0); + auto aNextScript = aEditEngine.GetScriptType({ 0, i, 0, i }); + if (aNextScript != aScript || i == maScriptText.getLength()) + maScriptChanges.emplace_back(aScript, i); + aScript = aNextScript; } } @@ -290,21 +268,21 @@ void FontPrevWin_Impl::CheckScript() Size FontPrevWin_Impl::CalcTextSize(vcl::RenderContext& rRenderContext, OutputDevice const * _pPrinter, const SvxFont& rInFont) { - sal_uInt16 nScript; + SvtScriptType aScript; sal_uInt16 nIdx = 0; sal_Int32 nStart = 0; sal_Int32 nEnd; - size_t nCnt = maScriptChg.size(); + size_t nCnt = maScriptChanges.size(); if (nCnt) { - nEnd = maScriptChg[nIdx]; - nScript = maScriptType[nIdx]; + nEnd = maScriptChanges[nIdx].changePos; + aScript = maScriptChanges[nIdx].scriptType; } else { nEnd = maText.getLength(); - nScript = css::i18n::ScriptType::LATIN; + aScript = SvtScriptType::LATIN; } tools::Long nTxtWidth = 0; tools::Long nCJKHeight = 0; @@ -316,24 +294,24 @@ Size FontPrevWin_Impl::CalcTextSize(vcl::RenderContext& rRenderContext, OutputDe do { - const SvxFont& rFont = (nScript == css::i18n::ScriptType::ASIAN) ? + const SvxFont& rFont = (aScript == SvtScriptType::ASIAN) ? maCJKFont : - ((nScript == css::i18n::ScriptType::COMPLEX) ? + ((aScript == SvtScriptType::COMPLEX) ? maCTLFont : rInFont); tools::Long nWidth = rFont.GetTextSize(*_pPrinter, maText, nStart, nEnd - nStart).Width(); - if (nIdx >= maTextWidth.size()) + if (nIdx >= maScriptChanges.size()) break; - maTextWidth[nIdx++] = nWidth; + maScriptChanges[nIdx++].textWidth = nWidth; nTxtWidth += nWidth; - switch (nScript) + switch (aScript) { - case css::i18n::ScriptType::ASIAN: + case SvtScriptType::ASIAN: calcFontHeightAnyAscent(rRenderContext, maCJKFont, nCJKHeight, nCJKAscent); break; - case css::i18n::ScriptType::COMPLEX: + case SvtScriptType::COMPLEX: calcFontHeightAnyAscent(rRenderContext, maCTLFont, nCTLHeight, nCTLAscent); break; default: @@ -343,8 +321,8 @@ Size FontPrevWin_Impl::CalcTextSize(vcl::RenderContext& rRenderContext, OutputDe if (nEnd < maText.getLength() && nIdx < nCnt) { nStart = nEnd; - nEnd = maScriptChg[nIdx]; - nScript = maScriptType[nIdx]; + nEnd = maScriptChanges[nIdx].changePos; + aScript = maScriptChanges[nIdx].scriptType; } else break; @@ -383,38 +361,39 @@ Size FontPrevWin_Impl::CalcTextSize(vcl::RenderContext& rRenderContext, OutputDe void FontPrevWin_Impl::DrawPrev(vcl::RenderContext& rRenderContext, Printer* _pPrinter, Point &rPt, const SvxFont& rInFont) { vcl::Font aOldFont = _pPrinter->GetFont(); - sal_uInt16 nScript; + SvtScriptType aScript; sal_uInt16 nIdx = 0; sal_Int32 nStart = 0; sal_Int32 nEnd; - size_t nCnt = maScriptChg.size(); + size_t nCnt = maScriptChanges.size(); + if (nCnt) { - nEnd = maScriptChg[nIdx]; - nScript = maScriptType[nIdx]; + nEnd = maScriptChanges[nIdx].changePos; + aScript = maScriptChanges[nIdx].scriptType; } else { nEnd = maText.getLength(); - nScript = css::i18n::ScriptType::LATIN; + aScript = SvtScriptType::LATIN; } do { - const SvxFont& rFont = (nScript == css::i18n::ScriptType::ASIAN) + const SvxFont& rFont = (aScript == SvtScriptType::ASIAN) ? maCJKFont - : ((nScript == css::i18n::ScriptType::COMPLEX) + : ((aScript == SvtScriptType::COMPLEX) ? maCTLFont : rInFont); _pPrinter->SetFont(rFont); rFont.DrawPrev(&rRenderContext, _pPrinter, rPt, maText, nStart, nEnd - nStart); - rPt.AdjustX(maTextWidth[nIdx++] ); + rPt.AdjustX(maScriptChanges[nIdx++].textWidth); if (nEnd < maText.getLength() && nIdx < nCnt) { nStart = nEnd; - nEnd = maScriptChg[nIdx]; - nScript = maScriptType[nIdx]; + nEnd = maScriptChanges[nIdx].changePos; + aScript = maScriptChanges[nIdx].scriptType; } else break; commit 85aac9d9b14d5af9700b34237cc9b0c82044cdbc Author: Khaled Hosny <kha...@aliftype.com> AuthorDate: Thu Dec 15 22:51:54 2022 +0200 Commit: خالد حسني <kha...@aliftype.com> CommitDate: Fri Dec 16 18:38:46 2022 +0000 tdf#152460: Improve script handling in style previews Use Edit Engine to get the script types instead of trying to duplicate (poorly) its behaviour. This has the advantage of handling weak characters and digits properly and is closer to what would happen in the document itself. We probably should go further and render the preview entirely using Edit Engine (since the current code fails short of proper bidi handling), but this is a bigger change. Change-Id: I79b28067d80f66087e1d5e9399ba1a513de96c8a Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144283 Tested-by: Jenkins Reviewed-by: خالد حسني <kha...@aliftype.com> (cherry picked from commit bfecacb2487ba9470600e6f64056d9b1816ee96b) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/144271 diff --git a/svx/inc/CommonStylePreviewRenderer.hxx b/svx/inc/CommonStylePreviewRenderer.hxx index cb0563be15c4..0e99421b4bfe 100644 --- a/svx/inc/CommonStylePreviewRenderer.hxx +++ b/svx/inc/CommonStylePreviewRenderer.hxx @@ -12,6 +12,7 @@ #include <optional> #include <vector> +#include <editeng/editeng.hxx> #include <editeng/svxfont.hxx> #include <sfx2/objsh.hxx> #include <sfx2/StylePreviewRenderer.hxx> @@ -19,9 +20,6 @@ #include <tools/color.hxx> #include <tools/gen.hxx> -#include <com/sun/star/uno/Reference.h> -#include <com/sun/star/i18n/BreakIterator.hpp> - class OutputDevice; class SfxStyleSheetBase; @@ -41,13 +39,12 @@ class CommonStylePreviewRenderer final : public sfx2::StylePreviewRenderer tools::Long mnBaseLine; OUString maStyleName; OUString maScriptText; - css::uno::Reference<css::i18n::XBreakIterator> mxBreak; struct ScriptInfo { tools::Long textWidth; - sal_uInt16 scriptType; + SvtScriptType scriptType; sal_Int32 changePos; - ScriptInfo(sal_uInt16 scrptType, sal_Int32 position) + ScriptInfo(SvtScriptType scrptType, sal_Int32 position) : textWidth(0) , scriptType(scrptType) , changePos(position) diff --git a/svx/source/styles/CommonStylePreviewRenderer.cxx b/svx/source/styles/CommonStylePreviewRenderer.cxx index 5f12d209f0bf..9ddf3cf15017 100644 --- a/svx/source/styles/CommonStylePreviewRenderer.cxx +++ b/svx/source/styles/CommonStylePreviewRenderer.cxx @@ -29,6 +29,7 @@ #include <editeng/contouritem.hxx> #include <editeng/colritem.hxx> #include <editeng/crossedoutitem.hxx> +#include <editeng/editeng.hxx> #include <editeng/emphasismarkitem.hxx> #include <editeng/postitem.hxx> #include <editeng/shdditem.hxx> @@ -39,9 +40,6 @@ #include <editeng/editids.hrc> -#include <comphelper/processfactory.hxx> -#include <com/sun/star/i18n/ScriptType.hpp> - using namespace css; namespace svx @@ -235,7 +233,7 @@ void CommonStylePreviewRenderer::CalcRenderSize() mnBaseLine = 0; mnHeight = 0; - sal_uInt16 nScript; + SvtScriptType aScript; sal_uInt16 nIdx = 0; sal_Int32 nStart = 0; sal_Int32 nEnd; @@ -244,19 +242,19 @@ void CommonStylePreviewRenderer::CalcRenderSize() if (nCnt) { nEnd = maScriptChanges[nIdx].changePos; - nScript = maScriptChanges[nIdx].scriptType; + aScript = maScriptChanges[nIdx].scriptType; } else { nEnd = rText.getLength(); - nScript = css::i18n::ScriptType::LATIN; + aScript = SvtScriptType::LATIN; } do { - auto oFont = (nScript == css::i18n::ScriptType::ASIAN) ? + auto oFont = (aScript == SvtScriptType::ASIAN) ? m_oCJKFont : - ((nScript == css::i18n::ScriptType::COMPLEX) ? + ((aScript == SvtScriptType::COMPLEX) ? m_oCTLFont : m_oFont); @@ -287,7 +285,7 @@ void CommonStylePreviewRenderer::CalcRenderSize() { nStart = nEnd; nEnd = maScriptChanges[nIdx].changePos; - nScript = maScriptChanges[nIdx].scriptType; + aScript = maScriptChanges[nIdx].scriptType; } else break; @@ -351,7 +349,7 @@ bool CommonStylePreviewRenderer::render(const tools::Rectangle& aRectangle, Rend aFontDrawPosition.AdjustY((aRectangle.GetHeight() - mnHeight) / 2 ); } - sal_uInt16 nScript; + SvtScriptType aScript; sal_uInt16 nIdx = 0; sal_Int32 nStart = 0; sal_Int32 nEnd; @@ -359,19 +357,19 @@ bool CommonStylePreviewRenderer::render(const tools::Rectangle& aRectangle, Rend if (nCnt) { nEnd = maScriptChanges[nIdx].changePos; - nScript = maScriptChanges[nIdx].scriptType; + aScript = maScriptChanges[nIdx].scriptType; } else { nEnd = rText.getLength(); - nScript = css::i18n::ScriptType::LATIN; + aScript = SvtScriptType::LATIN; } do { - auto oFont = (nScript == css::i18n::ScriptType::ASIAN) + auto oFont = (aScript == SvtScriptType::ASIAN) ? m_oCJKFont - : ((nScript == css::i18n::ScriptType::COMPLEX) + : ((aScript == SvtScriptType::COMPLEX) ? m_oCTLFont : m_oFont); @@ -392,7 +390,7 @@ bool CommonStylePreviewRenderer::render(const tools::Rectangle& aRectangle, Rend { nStart = nEnd; nEnd = maScriptChanges[nIdx].changePos; - nScript = maScriptChanges[nIdx].scriptType; + aScript = maScriptChanges[nIdx].scriptType; } else break; @@ -413,30 +411,16 @@ void CommonStylePreviewRenderer::CheckScript() maScriptText = maStyleName; maScriptChanges.clear(); - if (!mxBreak.is()) - { - auto xContext = comphelper::getProcessComponentContext(); - mxBreak = css::i18n::BreakIterator::create(xContext); - } - - sal_Int16 nScript = mxBreak->getScriptType(maStyleName, 0); - sal_Int32 nChg = 0; - if (css::i18n::ScriptType::WEAK == nScript) - { - nChg = mxBreak->endOfScript(maStyleName, nChg, nScript); - if (nChg < maStyleName.getLength()) - nScript = mxBreak->getScriptType(maStyleName, nChg); - else - nScript = css::i18n::ScriptType::LATIN; - } + auto aEditEngine = EditEngine(nullptr); + aEditEngine.SetText(maScriptText); - while (true) + auto aScript = aEditEngine.GetScriptType({ 0, 0, 0, 0 }); + for (sal_Int32 i = 1; i <= maScriptText.getLength(); i++) { - nChg = mxBreak->endOfScript(maStyleName, nChg, nScript); - maScriptChanges.emplace_back(nScript, nChg); - if (nChg >= maStyleName.getLength() || nChg < 0) - break; - nScript = mxBreak->getScriptType(maStyleName, nChg); + auto aNextScript = aEditEngine.GetScriptType({ 0, i, 0, i }); + if (aNextScript != aScript || i == maScriptText.getLength()) + maScriptChanges.emplace_back(aScript, i); + aScript = aNextScript; } } diff --git a/svx/source/tbxctrls/tbcontrl.cxx b/svx/source/tbxctrls/tbcontrl.cxx index 8df461642233..18b6eab37c62 100644 --- a/svx/source/tbxctrls/tbcontrl.cxx +++ b/svx/source/tbxctrls/tbcontrl.cxx @@ -104,10 +104,7 @@ #include <comphelper/lok.hxx> #include <tools/json_writer.hxx> -#include <com/sun/star/uno/Reference.h> -#include <com/sun/star/i18n/BreakIterator.hpp> -#include <comphelper/processfactory.hxx> -#include <com/sun/star/i18n/ScriptType.hpp> +#include <editeng/editeng.hxx> #define MAX_MRU_FONTNAME_ENTRIES 5 @@ -126,9 +123,9 @@ namespace struct ScriptInfo { tools::Long textWidth; - sal_uInt16 scriptType; + SvtScriptType scriptType; sal_Int32 changePos; - ScriptInfo(sal_uInt16 scrptType, sal_Int32 position) + ScriptInfo(SvtScriptType scrptType, sal_Int32 position) : textWidth(0) , scriptType(scrptType) , changePos(position) @@ -209,8 +206,6 @@ private: std::optional<SvxFont> m_oCJKFont; std::optional<SvxFont> m_oCTLFont; - css::uno::Reference<css::i18n::XBreakIterator> mxBreak; - DECL_LINK(SelectHdl, weld::ComboBox&, void); DECL_LINK(KeyInputHdl, const KeyEvent&, bool); DECL_LINK(ActivateHdl, weld::ComboBox&, bool); @@ -224,7 +219,6 @@ private: void Select(bool bNonTravelSelect); - std::vector<ScriptInfo> CheckScript(const OUString &rStyleName); tools::Rectangle CalcBoundRect(vcl::RenderContext& rRenderContext, const OUString &rStyleName, std::vector<ScriptInfo>& rScriptChanges, double fRatio = 1); protected: @@ -1126,46 +1120,35 @@ void SvxStyleBox_Impl::SetOptimalSize() SetSizePixel(get_preferred_size()); } -std::vector<ScriptInfo> SvxStyleBox_Base::CheckScript(const OUString &rStyleName) +namespace +{ +std::vector<ScriptInfo> CheckScript(const OUString &rStyleName) { assert(!rStyleName.isEmpty()); // must have a preview text here! std::vector<ScriptInfo> aScriptChanges; - if (!mxBreak.is()) - { - auto xContext = comphelper::getProcessComponentContext(); - mxBreak = css::i18n::BreakIterator::create(xContext); - } - - sal_Int16 nScript = mxBreak->getScriptType(rStyleName, 0); - sal_Int32 nChg = 0; - if (css::i18n::ScriptType::WEAK == nScript) - { - nChg = mxBreak->endOfScript(rStyleName, nChg, nScript); - if (nChg < rStyleName.getLength()) - nScript = mxBreak->getScriptType(rStyleName, nChg); - else - nScript = css::i18n::ScriptType::LATIN; - } + auto aEditEngine = EditEngine(nullptr); + aEditEngine.SetText(rStyleName); - while (true) + auto aScript = aEditEngine.GetScriptType({ 0, 0, 0, 0 }); + for (sal_Int32 i = 1; i <= rStyleName.getLength(); i++) { - nChg = mxBreak->endOfScript(rStyleName, nChg, nScript); - aScriptChanges.emplace_back(nScript, nChg); - if (nChg >= rStyleName.getLength() || nChg < 0) - break; - nScript = mxBreak->getScriptType(rStyleName, nChg); + auto aNextScript = aEditEngine.GetScriptType({ 0, i, 0, i }); + if (aNextScript != aScript || i == rStyleName.getLength()) + aScriptChanges.emplace_back(aScript, i); + aScript = aNextScript; } return aScriptChanges; } +} tools::Rectangle SvxStyleBox_Base::CalcBoundRect(vcl::RenderContext& rRenderContext, const OUString &rStyleName, std::vector<ScriptInfo>& rScriptChanges, double fRatio) { tools::Rectangle aTextRect; - sal_uInt16 nScript; + SvtScriptType aScript; sal_uInt16 nIdx = 0; sal_Int32 nStart = 0; sal_Int32 nEnd; @@ -1174,19 +1157,19 @@ tools::Rectangle SvxStyleBox_Base::CalcBoundRect(vcl::RenderContext& rRenderCont if (nCnt) { nEnd = rScriptChanges[nIdx].changePos; - nScript = rScriptChanges[nIdx].scriptType; + aScript = rScriptChanges[nIdx].scriptType; } else { nEnd = rStyleName.getLength(); - nScript = css::i18n::ScriptType::LATIN; + aScript = SvtScriptType::LATIN; } do { - auto oFont = (nScript == css::i18n::ScriptType::ASIAN) ? + auto oFont = (aScript == SvtScriptType::ASIAN) ? m_oCJKFont : - ((nScript == css::i18n::ScriptType::COMPLEX) ? + ((aScript == SvtScriptType::COMPLEX) ? m_oCTLFont : m_oFont); @@ -1222,7 +1205,7 @@ tools::Rectangle SvxStyleBox_Base::CalcBoundRect(vcl::RenderContext& rRenderCont { nStart = nEnd; nEnd = rScriptChanges[nIdx].changePos; - nScript = rScriptChanges[nIdx].scriptType; + aScript = rScriptChanges[nIdx].scriptType; } else break; @@ -1248,29 +1231,31 @@ void SvxStyleBox_Base::UserDrawEntry(vcl::RenderContext& rRenderContext, const t else aPos.AdjustY((rRect.GetHeight() - rTextRect.Bottom()) / 2); - sal_uInt16 nScript; + SvtScriptType aScript; sal_uInt16 nIdx = 0; sal_Int32 nStart = 0; sal_Int32 nEnd; size_t nCnt = rScriptChanges.size(); + if (nCnt) { nEnd = rScriptChanges[nIdx].changePos; - nScript = rScriptChanges[nIdx].scriptType; + aScript = rScriptChanges[nIdx].scriptType; } else { nEnd = rStyleName.getLength(); - nScript = css::i18n::ScriptType::LATIN; + aScript = SvtScriptType::LATIN; } + do { - auto oFont = (nScript == css::i18n::ScriptType::ASIAN) - ? m_oCJKFont - : ((nScript == css::i18n::ScriptType::COMPLEX) - ? m_oCTLFont - : m_oFont); + auto oFont = (aScript == SvtScriptType::ASIAN) ? + m_oCJKFont : + ((aScript == SvtScriptType::COMPLEX) ? + m_oCTLFont : + m_oFont); rRenderContext.Push(vcl::PushFlags::FONT); @@ -1296,7 +1281,7 @@ void SvxStyleBox_Base::UserDrawEntry(vcl::RenderContext& rRenderContext, const t { nStart = nEnd; nEnd = rScriptChanges[nIdx].changePos; - nScript = rScriptChanges[nIdx].scriptType; + aScript = rScriptChanges[nIdx].scriptType; } else break;