[Libreoffice-commits] core.git: vcl/inc vcl/source
Khaled Hosny
khaledhosny at eglug.org
Sat Nov 26 19:34:17 UTC 2016
vcl/inc/CommonSalLayout.hxx | 17 +-
vcl/inc/sallayout.hxx | 8 -
vcl/source/gdi/sallayout.cxx | 279 -------------------------------------------
3 files changed, 15 insertions(+), 289 deletions(-)
New commits:
commit 3dab9849115284f9a126356e2354ad7fb8557663
Author: Khaled Hosny <khaledhosny at eglug.org>
Date: Sat Nov 26 17:29:16 2016 +0200
More dead code
Change-Id: I518b56566cdf1eceee7a868b9bf4ab4f6e498f98
Reviewed-on: https://gerrit.libreoffice.org/31234
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Khaled Hosny <khaledhosny at eglug.org>
diff --git a/vcl/inc/CommonSalLayout.hxx b/vcl/inc/CommonSalLayout.hxx
index e5345d7..05ea342 100644
--- a/vcl/inc/CommonSalLayout.hxx
+++ b/vcl/inc/CommonSalLayout.hxx
@@ -61,6 +61,8 @@ class CommonSalLayout : public GenericSalLayout
hb_set_t* mpVertGlyphs;
bool IsVerticalAlternate(hb_codepoint_t nGlyphIndex);
+ void SetNeedFallback(ImplLayoutArgs&, sal_Int32, bool);
+
public:
#if defined(_WIN32)
explicit CommonSalLayout(HDC, WinFontInstance&, const WinFontFace&);
@@ -74,16 +76,15 @@ public:
#endif
virtual void InitFont() const override;
- void SetNeedFallback(ImplLayoutArgs&, sal_Int32, bool);
- void AdjustLayout(ImplLayoutArgs&) override;
- bool LayoutText(ImplLayoutArgs&) override;
- void DrawText(SalGraphics&) const override;
- std::shared_ptr<vcl::TextLayoutCache> CreateTextLayoutCache(OUString const&) const override;
+ void AdjustLayout(ImplLayoutArgs&) final override;
+ bool LayoutText(ImplLayoutArgs&) final override;
+ void DrawText(SalGraphics&) const final override;
+ std::shared_ptr<vcl::TextLayoutCache> CreateTextLayoutCache(OUString const&) const final override;
- virtual bool GetCharWidths(DeviceCoordinate* pCharWidths) const override;
- virtual void ApplyDXArray(ImplLayoutArgs&) override;
+ bool GetCharWidths(DeviceCoordinate* pCharWidths) const final override;
+ void ApplyDXArray(ImplLayoutArgs&) final override;
- virtual bool IsKashidaPosValid(int nCharPos) const override;
+ bool IsKashidaPosValid(int nCharPos) const final override;
};
enum class VerticalOrientation {
diff --git a/vcl/inc/sallayout.hxx b/vcl/inc/sallayout.hxx
index 3fab81f..fddc8d1 100644
--- a/vcl/inc/sallayout.hxx
+++ b/vcl/inc/sallayout.hxx
@@ -332,10 +332,8 @@ public:
// used by layout engines
void AppendGlyph( const GlyphItem& );
void Reserve(int size) { m_GlyphItems.reserve(size + 1); }
- virtual void AdjustLayout( ImplLayoutArgs& ) override;
- virtual void ApplyDXArray( ImplLayoutArgs& );
- void Justify( DeviceCoordinate nNewWidth );
- void KashidaJustify( long nIndex, int nWidth );
+ virtual void ApplyDXArray(ImplLayoutArgs&) = 0;
+ void Justify(DeviceCoordinate nNewWidth);
void ApplyAsianKerning(const OUString& rStr);
void SortGlyphItems();
@@ -359,7 +357,7 @@ protected:
virtual void DropGlyph( int nStart ) override;
virtual void Simplify( bool bIsBase ) override;
- virtual bool GetCharWidths( DeviceCoordinate* pCharWidths ) const;
+ virtual bool GetCharWidths(DeviceCoordinate* pCharWidths) const = 0;
std::vector<GlyphItem> m_GlyphItems;
diff --git a/vcl/source/gdi/sallayout.cxx b/vcl/source/gdi/sallayout.cxx
index 63a65e6..d879df0 100644
--- a/vcl/source/gdi/sallayout.cxx
+++ b/vcl/source/gdi/sallayout.cxx
@@ -751,6 +751,9 @@ bool SalLayout::GetBoundRect( SalGraphics& rSalGraphics, Rectangle& rRect ) cons
return bRet;
}
+// FIXME: This function is just broken, it assumes any glyph at index 3 in the
+// font is space, which though common is not a hard requirement and not the
+// only glyph for space characters. Fix the call sites and fix them.
bool SalLayout::IsSpacingGlyph( sal_GlyphId nGlyph )
{
bool bRet = false;
@@ -784,80 +787,6 @@ void GenericSalLayout::AppendGlyph( const GlyphItem& rGlyphItem )
m_GlyphItems.push_back(rGlyphItem);
}
-bool GenericSalLayout::GetCharWidths( DeviceCoordinate* pCharWidths ) const
-{
- // initialize character extents buffer
- int nCharCount = mnEndCharPos - mnMinCharPos;
- for( int n = 0; n < nCharCount; ++n )
- pCharWidths[n] = 0;
-
- // determine cluster extents
- for( std::vector<GlyphItem>::const_iterator pGlyphIter = m_GlyphItems.begin(), end = m_GlyphItems.end(); pGlyphIter != end ; ++pGlyphIter)
- {
- // use cluster start to get char index
- if( !pGlyphIter->IsClusterStart() )
- continue;
-
- int n = pGlyphIter->mnCharPos;
- if( n >= mnEndCharPos )
- continue;
- n -= mnMinCharPos;
- if( n < 0 )
- continue;
-
- // left glyph in cluster defines default extent
- long nXPosMin = pGlyphIter->maLinearPos.X();
- long nXPosMax = nXPosMin + pGlyphIter->mnNewWidth;
-
- // calculate right x-position for this glyph cluster
- // break if no more glyphs in layout
- // break at next glyph cluster start
- while( (pGlyphIter+1 != end) && !pGlyphIter[1].IsClusterStart() )
- {
- // advance to next glyph in cluster
- ++pGlyphIter;
-
- if( pGlyphIter->IsDiacritic() )
- continue; // ignore diacritics
- // get leftmost x-extent of this glyph
- long nXPos = pGlyphIter->maLinearPos.X();
- if( nXPosMin > nXPos )
- nXPosMin = nXPos;
-
- // get rightmost x-extent of this glyph
- nXPos += pGlyphIter->mnNewWidth;
- if( nXPosMax < nXPos )
- nXPosMax = nXPos;
- }
-
- // when the current cluster overlaps with the next one assume
- // rightmost cluster edge is the leftmost edge of next cluster
- // for clusters that do not have x-sorted glyphs
- // TODO: avoid recalculation of left bound in next cluster iteration
- for( std::vector<GlyphItem>::const_iterator pN = pGlyphIter; ++pN != end; )
- {
- if( pN->IsClusterStart() )
- break;
- if( pN->IsDiacritic() )
- continue; // ignore diacritics
- if( nXPosMax > pN->maLinearPos.X() )
- nXPosMax = pN->maLinearPos.X();
- }
- if( nXPosMax < nXPosMin )
- nXPosMin = nXPosMax = 0;
-
- // character width is sum of glyph cluster widths
- pCharWidths[n] += nXPosMax - nXPosMin;
- }
-
- // TODO: distribute the cluster width proportionally to the characters
- // clusters (e.g. ligatures) correspond to more than one char index,
- // so some character widths are still uninitialized. This is solved
- // by setting the first charwidth of the cluster to the cluster width
-
- return true;
-}
-
DeviceCoordinate GenericSalLayout::FillDXArray( DeviceCoordinate* pCharWidths ) const
{
if( pCharWidths )
@@ -892,157 +821,6 @@ DeviceCoordinate GenericSalLayout::GetTextWidth() const
return nWidth;
}
-void GenericSalLayout::AdjustLayout( ImplLayoutArgs& rArgs )
-{
- SalLayout::AdjustLayout( rArgs );
-
- if( rArgs.mpDXArray )
- ApplyDXArray( rArgs );
- else if( rArgs.mnLayoutWidth )
- Justify( rArgs.mnLayoutWidth );
-}
-
-// This DXArray thing is one of the stupidest ideas I have ever seen (I've been
-// told that it probably a one-to-one mapping of some Windows 3.1 API, which is
-// telling).
-
-// Note: That would be the EMRTEXT structure, which is part of the EMF spec (see
-// [MS-EMF] section 2.2.5. Basically the field in question is OutputDx, which is:
-//
-// "An array of 32-bit unsigned integers that specify the output spacing
-// between the origins of adjacent character cells in logical units."
-//
-// This obviously makes sense for ASCII text (the EMR_EXTTEXTOUTA record), but it
-// doesn't make sense for Unicode text (the EMR_EXTTEXTOUTW record) because it is
-// mapping character codes to output spacing, which obviously can cause problems
-// with CTL. MANY of our concepts are based around Microsoft data structures, this
-// is obviously one of them and we probably need a rethink about how we go about
-// this.
-
-// To justify a text string, Writer calls OutputDevice::GetTextArray()
-// to get an array that maps input characters (not glyphs) to their absolute
-// position, GetTextArray() in turn calls SalLayout::FillDXArray() to get an
-// array of character widths that it converts to absolute positions.
-
-// Writer would then apply justification adjustments to that array of absolute
-// character positions and return to OutputDevice, which eventually calls
-// ApplyDXArray(), which needs to extract the individual adjustments for each
-// character to apply it to corresponding glyphs, and since that information is
-// already lost it tries to do some heuristics to guess it again. Those
-// heuristics often fail, and have always been a source of all sorts of weird
-// text layout bugs, and instead of fixing the broken design a hack after hack
-// have been applied on top of it, making it a complete mess that nobody
-// understands.
-
-// As you can see by now, this is utterly stupid, why doesn't Writer just send
-// us the advance width transformations it wants to apply to each character directly
-// instead of this whole mess?
-
-void GenericSalLayout::ApplyDXArray( ImplLayoutArgs& rArgs )
-{
- if( m_GlyphItems.empty())
- return;
-
- // determine cluster boundaries and x base offset
- const int nCharCount = rArgs.mnEndCharPos - rArgs.mnMinCharPos;
- std::unique_ptr<int[]> const pLogCluster(new int[nCharCount]);
- size_t i;
- int n,p;
- long nBasePointX = -1;
- if( mnLayoutFlags & SalLayoutFlags::ForFallback )
- nBasePointX = 0;
- for(p = 0; p < nCharCount; ++p )
- pLogCluster[ p ] = -1;
-
- for( i = 0; i < m_GlyphItems.size(); ++i)
- {
- n = m_GlyphItems[i].mnCharPos - rArgs.mnMinCharPos;
- if( (n < 0) || (nCharCount <= n) )
- continue;
- if( pLogCluster[ n ] < 0 )
- pLogCluster[ n ] = i;
- if( nBasePointX < 0 )
- nBasePointX = m_GlyphItems[i].maLinearPos.X();
- }
- // retarget unresolved pLogCluster[n] to a glyph inside the cluster
- // TODO: better do it while the deleted-glyph markers are still there
- for( n = 0; n < nCharCount; ++n )
- if( (p = pLogCluster[n]) >= 0 )
- break;
- if( n >= nCharCount )
- return;
- for( n = 0; n < nCharCount; ++n )
- {
- if( pLogCluster[ n ] < 0 )
- pLogCluster[ n ] = p;
- else
- p = pLogCluster[ n ];
- }
-
- // calculate adjusted cluster widths
- std::unique_ptr<long[]> const pNewGlyphWidths(new long[m_GlyphItems.size()]);
- for( i = 0; i < m_GlyphItems.size(); ++i )
- pNewGlyphWidths[ i ] = 0;
-
- bool bRTL;
- for( int nCharPos = p = -1; rArgs.GetNextPos( &nCharPos, &bRTL ); )
- {
- n = nCharPos - rArgs.mnMinCharPos;
- if( (n < 0) || (nCharCount <= n) ) continue;
-
- if( pLogCluster[ n ] >= 0 )
- p = pLogCluster[ n ];
- if( p >= 0 )
- {
- long nDelta = rArgs.mpDXArray[ n ] ;
- if( n > 0 )
- nDelta -= rArgs.mpDXArray[ n-1 ];
- pNewGlyphWidths[ p ] += nDelta * mnUnitsPerPixel;
- }
- }
-
- // move cluster positions using the adjusted widths
- long nDelta = 0;
- long nNewPos = 0;
- for( i = 0; i < m_GlyphItems.size(); ++i)
- {
- if( m_GlyphItems[i].IsClusterStart() )
- {
- // calculate original and adjusted cluster width
- int nOldClusterWidth = m_GlyphItems[i].mnNewWidth - m_GlyphItems[i].mnXOffset;
- int nNewClusterWidth = pNewGlyphWidths[i];
- size_t j;
- for( j = i; ++j < m_GlyphItems.size(); )
- {
- if( m_GlyphItems[j].IsClusterStart() )
- break;
- if( !m_GlyphItems[j].IsDiacritic() ) // #i99367# ignore diacritics
- nOldClusterWidth += m_GlyphItems[j].mnNewWidth - m_GlyphItems[j].mnXOffset;
- nNewClusterWidth += pNewGlyphWidths[j];
- }
- const int nDiff = nNewClusterWidth - nOldClusterWidth;
-
- // adjust cluster glyph widths and positions
- nDelta = nBasePointX + (nNewPos - m_GlyphItems[i].maLinearPos.X());
- if( !m_GlyphItems[i].IsRTLGlyph() )
- {
- // for LTR case extend rightmost glyph in cluster
- m_GlyphItems[j - 1].mnNewWidth += nDiff;
- }
- else
- {
- // right align cluster in new space for RTL case
- m_GlyphItems[i].mnNewWidth += nDiff;
- nDelta += nDiff;
- }
-
- nNewPos += nNewClusterWidth;
- }
-
- m_GlyphItems[i].maLinearPos.X() += nDelta;
- }
-}
-
void GenericSalLayout::Justify( DeviceCoordinate nNewWidth )
{
nNewWidth *= mnUnitsPerPixel;
@@ -1160,57 +938,6 @@ void GenericSalLayout::ApplyAsianKerning(const OUString& rStr)
}
}
-void GenericSalLayout::KashidaJustify( long nKashidaIndex, int nKashidaWidth )
-{
- // TODO: reimplement method when container type for GlyphItems changes
-
- // skip if the kashida glyph in the font looks suspicious
- if( nKashidaWidth <= 0 )
- return;
-
- // calculate max number of needed kashidas
- int nKashidaCount = 0;
- for (std::vector<GlyphItem>::iterator pGlyphIter = m_GlyphItems.begin();
- pGlyphIter != m_GlyphItems.end(); ++pGlyphIter)
- {
- // only inject kashidas in RTL contexts
- if( !pGlyphIter->IsRTLGlyph() )
- continue;
- // no kashida-injection for blank justified expansion either
- if( IsSpacingGlyph( pGlyphIter->maGlyphId) )
- continue;
-
- // calculate gap, ignore if too small
- int nGapWidth = pGlyphIter->mnNewWidth - pGlyphIter->mnOrigWidth;
- // worst case is one kashida even for mini-gaps
- if( nGapWidth < nKashidaWidth )
- continue;
-
- nKashidaCount = 0;
- Point aPos = pGlyphIter->maLinearPos;
- aPos.X() -= nGapWidth; // cluster is already right aligned
- int const nCharPos = pGlyphIter->mnCharPos;
- std::vector<GlyphItem>::iterator pGlyphIter2 = pGlyphIter;
- for(; nGapWidth > nKashidaWidth; nGapWidth -= nKashidaWidth, ++nKashidaCount )
- {
- pGlyphIter2 = m_GlyphItems.insert(pGlyphIter2, GlyphItem(nCharPos, nKashidaIndex, aPos,
- GlyphItem::IS_IN_CLUSTER|GlyphItem::IS_RTL_GLYPH, nKashidaWidth ));
- ++pGlyphIter2;
- aPos.X() += nKashidaWidth;
- }
-
- // fixup rightmost kashida for gap remainder
- if( nGapWidth > 0 )
- {
- pGlyphIter2 = m_GlyphItems.insert(pGlyphIter2, GlyphItem(nCharPos, nKashidaIndex, aPos,
- GlyphItem::IS_IN_CLUSTER|GlyphItem::IS_RTL_GLYPH, nKashidaCount ? nGapWidth : nGapWidth/2 ));
- ++pGlyphIter2;
- aPos.X() += nGapWidth;
- }
- pGlyphIter = pGlyphIter2;
- }
-}
-
void GenericSalLayout::GetCaretPositions( int nMaxIndex, long* pCaretXArray ) const
{
// initialize result array
More information about the Libreoffice-commits
mailing list