Subject: commit - Screen coordinates and lines
jskov@zoftcorp.adsl.dk
Date: Mon Apr 16 2001 - 08:15:04 CDT
I've made the changes I suggested in my previous mails. And I'm going
to commit them. As far as I can tell, the code works. I invite people
to have a look at the below patch and send suggestions for (or
implementations of) alternative ways around this problem. I'm fairly
sure this should be OK, though.
Rather interestingly, I noted that we're probably getting some pixel
dirt on the screen with lines rendering on top of each other, also
because of this problem. See the \fixme in the comments.
If you want to test out the change, see bug
http://www.abisource.com/bugzilla/show_bug.cgi?id=307. Scrolling down
with the old code will cause the cursor to get stuck by the first
picture.
Jesper
Index: fp_Column.cpp
===================================================================
RCS file: /cvsroot/abi/src/text/fmt/xp/fp_Column.cpp,v
retrieving revision 1.77
diff -u -5 -r1.77 fp_Column.cpp
--- fp_Column.cpp 2001/04/16 11:50:44 1.77
+++ fp_Column.cpp 2001/04/16 13:10:30
@@ -541,30 +541,72 @@
void fp_Column::setPrev(fp_Column*p)
{
m_pPrev = p;
}
+
+/*!
+ Layout lines in the column
+
+ This function iterates over the lines in the column and computes
+ their screen position from their accumulative heights in layout
+ units.
+
+ Since this code accumulates fractions of the conversion process, the
+ difference between Y positions of two lines may differ from the
+ pre-computed height of the upper line. This is due to simple
+ rounding errors and general lack of precision (screen coordinates
+ are integer while the computation yields fractions).
+
+ To make XY/position conversion precise, remove the gaps by updating
+ the line heights. Note that the line heights get updated next time
+ there's a line lookup - so this does not in any way affect layout,
+ only the precision of the XY/position conversion code.
+
+ \see fp_Line::setAssignedScreenHeight, fp_Line::recalcHeight
+*/
void fp_Column::layout(void)
{
UT_sint32 iYLayoutUnits = 0;
+ UT_sint32 iY = 0, iPrevY = 0;
double ScaleLayoutUnitsToScreen;
ScaleLayoutUnitsToScreen = (double)m_pG->getResolution() / UT_LAYOUT_UNITS;
UT_uint32 iCountLines = m_vecLines.getItemCount();
+ fp_Line *pLine, *pPrevLine = NULL;
for (UT_uint32 i=0; i < iCountLines; i++)
{
- fp_Line* pLine = (fp_Line*) m_vecLines.getNthItem(i);
-
+ pLine = (fp_Line*) m_vecLines.getNthItem(i);
+
UT_sint32 iLineHeightLayoutUnits = pLine->getHeightInLayoutUnits();
// UT_sint32 iLineMarginBefore = (i != 0) ? pLine->getMarginBefore() : 0;
UT_sint32 iLineMarginAfterLayoutUnits = pLine->getMarginAfterInLayoutUnits();
// iY += iLineMarginBefore;
- pLine->setY((int)(ScaleLayoutUnitsToScreen * iYLayoutUnits));
+ iY = (int)(ScaleLayoutUnitsToScreen * iYLayoutUnits);
+ pLine->setY(iY);
pLine->setYInLayoutUnits(iYLayoutUnits);
+
iYLayoutUnits += iLineHeightLayoutUnits;
iYLayoutUnits += iLineMarginAfterLayoutUnits;
+
+ // Update height of previous line now we know the gap between
+ // it and the current line.
+ if (pPrevLine)
+ {
+ pPrevLine->setAssignedScreenHeight(iY - iPrevY);
+ }
+ pPrevLine = pLine;
+ iPrevY = iY;
+ }
+
+ // Correct height position of the last line
+ if (pPrevLine)
+ {
+ iY = (int)(ScaleLayoutUnitsToScreen * iYLayoutUnits);
+ pPrevLine->setAssignedScreenHeight(iY - iPrevY);
}
+
UT_sint32 iNewHeight = (int)(ScaleLayoutUnitsToScreen * iYLayoutUnits);
if (m_iHeight == iNewHeight)
{
return;
Index: fp_Line.cpp
===================================================================
RCS file: /cvsroot/abi/src/text/fmt/xp/fp_Line.cpp,v
retrieving revision 1.95
diff -u -5 -r1.95 fp_Line.cpp
--- fp_Line.cpp 2001/04/16 05:42:31 1.95
+++ fp_Line.cpp 2001/04/16 13:10:33
@@ -469,10 +469,53 @@
xoff = my_xoff + pRun->getX();
yoff = my_yoff + pRun->getY();
}
+/*!
+ Set height assigned to line on screen
+ \param iHeight Height in screen units
+
+ While recalcHeight computes the height of the line as it will render
+ on the screen, the fp_Column does the actual line layout and does so
+ with greater accuracy. In particular, the line may be assigned a
+ different height on the screen than what it asked for.
+
+ This function allows the line representation to reflect the actual
+ screen layout size, which improves the precision of XY/position
+ conversion functions.
+
+ \note This function is quite intentionally <b>not</b> called
+ setHeight. It should <b>only</b> be called from
+ fp_Column::layout.
+
+ \see fp_Column::layout
+*/
+void fp_Line::setAssignedScreenHeight(UT_sint32 iHeight)
+{
+ m_iHeight = iHeight;
+}
+
+/*!
+ Compute the height of the line
+
+ Note that while the line is asked to provide height/width and
+ computes this based on its content Runs, it may later be assigned
+ additional screen estate by having its height changed. That does not
+ affect or override layout details, but increases precision of
+ XY/position conversions.
+
+ \fixme I originally put in an assertion that checked that the line
+ was only ever asked to grow in size. But that fired a lot, so
+ it had to be removed. This suggests that we actually try to
+ render stuff to closely on screen - the fp_Line::recalcHeight
+ function should probably be fixed to round height and widths
+ up always. But it gets its data from Runs, so this is not
+ where the problem should be fixed.
+
+ \see fp_Column::layout, fp_Line::setAssignedScreenHeight
+*/
void fp_Line::recalcHeight()
{
UT_sint32 count = m_vecRuns.getItemCount();
UT_sint32 i;
@@ -518,35 +561,30 @@
UT_sint32 iNewHeight = iMaxAscent + iMaxDescent;
UT_sint32 iNewHeightLayoutUnits = iMaxAscentLayoutUnits + iMaxDescentLayoutUnits;
UT_sint32 iNewAscent = iMaxAscent;
UT_sint32 iNewDescent = iMaxDescent;
+ // adjust line height to include leading
+ double dLineSpace, dLineSpaceLayout;
+ fl_BlockLayout::eSpacingPolicy eSpacing;
+ m_pBlock->getLineSpacing(dLineSpace, dLineSpaceLayout, eSpacing);
+
+ if (eSpacing == fl_BlockLayout::spacing_EXACT)
+ {
+ iNewHeight = (UT_sint32) dLineSpace;
+ iNewHeightLayoutUnits = (UT_sint32) dLineSpaceLayout;
+ }
+ else if (eSpacing == fl_BlockLayout::spacing_ATLEAST)
+ {
+ iNewHeight = UT_MAX(iNewHeight, (UT_sint32) dLineSpace);
+ iNewHeightLayoutUnits = UT_MAX(iNewHeightLayoutUnits, (UT_sint32) dLineSpaceLayout);
+ }
+ else
{
- // adjust line height to include leading
- double dLineSpace, dLineSpaceLayout;
- fl_BlockLayout::eSpacingPolicy eSpacing;
- m_pBlock->getLineSpacing(dLineSpace, dLineSpaceLayout, eSpacing);
-
- if (eSpacing == fl_BlockLayout::spacing_EXACT)
- {
- iNewHeight = (UT_sint32) dLineSpace;
-
- iNewHeightLayoutUnits = (UT_sint32) dLineSpaceLayout;
-
- }
- else if (eSpacing == fl_BlockLayout::spacing_ATLEAST)
- {
- iNewHeight = UT_MAX(iNewHeight, (UT_sint32) dLineSpace);
-
- iNewHeightLayoutUnits = UT_MAX(iNewHeightLayoutUnits, (UT_sint32) dLineSpaceLayout);
- }
- else
- {
- // multiple
- iNewHeight = (UT_sint32) (iNewHeight * dLineSpace);
- iNewHeightLayoutUnits = (UT_sint32) (iNewHeightLayoutUnits * dLineSpaceLayout);
- }
+ // multiple
+ iNewHeight = (UT_sint32) (iNewHeight * dLineSpace);
+ iNewHeightLayoutUnits = (UT_sint32) (iNewHeightLayoutUnits * dLineSpaceLayout);
}
if (
(iOldHeight != iNewHeight)
|| (iOldAscent != iNewAscent)
Index: fp_Line.h
===================================================================
RCS file: /cvsroot/abi/src/text/fmt/xp/fp_Line.h,v
retrieving revision 1.39
diff -u -5 -r1.39 fp_Line.h
--- fp_Line.h 2001/03/25 08:04:31 1.39
+++ fp_Line.h 2001/04/16 13:10:34
@@ -89,10 +89,12 @@
inline UT_sint32 getAscent(void) const { return m_iAscent; }
inline UT_sint32 getDescent(void) const { return m_iDescent; }
inline UT_sint32 getColumnGap(void) const { return m_pContainer->getColumnGap(); }
+ void setAssignedScreenHeight(UT_sint32);
+
void setMaxWidth(UT_sint32);
void setMaxWidthInLayoutUnits(UT_sint32);
void setX(UT_sint32);
void setXInLayoutUnits(UT_sint32);
void setY(UT_sint32);
Index: fv_View.cpp
===================================================================
RCS file: /cvsroot/abi/src/text/fmt/xp/fv_View.cpp,v
retrieving revision 1.429
diff -u -5 -r1.429 fv_View.cpp
--- fv_View.cpp 2001/04/15 23:25:40 1.429
+++ fv_View.cpp 2001/04/16 13:10:49
@@ -3578,28 +3578,32 @@
// Signal PieceTable Changes have finished
m_pDoc->notifyPieceTableChangeEnd();
}
+/*!
+ Move insertion point to previous or next line
+ \param bNext True if moving to next line
+
+ This function moves the IP up or down one line, attempting to get as
+ close as possible to the prior "sticky" x position. The notion of
+ "next" is strictly physical, not logical.
+
+ For example, instead of always moving from the last line of one
+ block to the first line of the next, you might wind up skipping over
+ a bunch of blocks to wind up in the first line of the second column.
+*/
void FV_View::_moveInsPtNextPrevLine(bool bNext)
{
UT_sint32 xPoint;
UT_sint32 yPoint;
UT_sint32 iPointHeight;
UT_sint32 iLineHeight;
UT_sint32 xPoint2;
UT_sint32 yPoint2;
bool bDirection;
- /*
- This function moves the IP up or down one line, attempting to get
- as close as possible to the prior "sticky" x position. The notion
- of "next" is strictly physical, not logical.
-
- For example, instead of always moving from the last line of one block
- to the first line of the next, you might wind up skipping over a
- bunch of blocks to wind up in the first line of the second column.
- */
+
UT_sint32 xOldSticky = m_xPointSticky;
// first, find the line we are on now
UT_uint32 iOldPoint = getPoint();
@@ -3633,12 +3637,11 @@
if (bNext)
{
if (pOldLine != pOldContainer->getLastLine())
{
// just move off this line
- // Sevior TODO the +2 is a work around. The problem is somewhere else
- yPoint += (iLineHeight + pOldLine->getMarginAfter()+2);
+ yPoint += (iLineHeight + pOldLine->getMarginAfter()+1);
}
else if (bDocSection && (((fp_Column*) (pOldSL->getLastContainer()))->getLeader() == pOldLeader))
{
// move to next section
fl_SectionLayout* pSL = pOldSL->getNext();
@@ -3669,12 +3672,11 @@
else
{
if (pOldLine != pOldContainer->getFirstLine())
{
// just move off this line
- // Sevior TODO the +2 is a work around. The problem is somewhere else
- yPoint -= (pOldLine->getMarginBefore() + 2);
+ yPoint -= (pOldLine->getMarginBefore() + 1);
}
else if (bDocSection && (pOldSL->getFirstContainer() == pOldLeader))
{
// move to prev section
fl_SectionLayout* pSL = pOldSL->getPrev();
This archive was generated by hypermail 2b25 : Mon Apr 16 2001 - 08:16:28 CDT