At some time it seemed like a good idea in breakRow() to return early
when the row was bound to be empty. It turns out that this creates two
symptoms:
* empty paragraphs will not have an end of paragraph marker
* since row width is not correctly computed in this case, caret ghosts
can appear in master.
This commit removes the oprimization and replace the do {} while()
construct to a straightforward while() {}.
Related to bug #10952.
The bug is the following: when selecting several paragraphs quickly
enough, some rows do not get selected.This is a consequence of the
removal of row crc, which lead to not taking into account the
selection status of the row in the decision to repaint.
The solution chosen here is to add a Row::change() helper function to
modify row members. This will set the Row changed status whenever the
value of the member changes.
When the caret is at end of row, if may happen that it is drawn after
the end of the row. In this case caret blinking will not work
properly. This patch extends the row background on the left and right
by Inset::TEXT_TO_INSET_OFFSET. This is only a hack that will not work
if the caret has a ridiculous width like 6.
Additionally, introduce some (disabled) debug code that numbers the
rows on screen by painting order.
Finally, make the code that detects whether the caret was in a given
row more precise (take boundary into account).
Fixes (mostly, see above) bug #10797.
This computation did not make sense anymore since we began to put the
contents in the Row object. The fact that it worked was a coincidence.
Instead, we set rows as changed() on creation and reset that once they
have been drawn. This will allow in the future for a finer definition
of what to redraw or not.
Also update the PAINTING_ANALYSIS document
It is wrong to compute this at paint time. In general, painting a row
should not require any access to a paragraph object, but we are far
from there now.
The goal of this commit is to ensure that a processUpdateFlags call
that requires no redraw will not override a previous one that did
require a redraw.
To this end, the semantics of the flag argument is now different: its
value is now OR'ed with a private update_flags_ variable. This
variable is only reset after the buffer view has actually been
redrawn.
A new Update::ForceRedraw flag has been added. It requires a full
redraw but no metrics computation. It is not used in the main code
(yet), but avoids to compute metrics repeatedly in consecutive
processUpdateFlags calls.
The process is now as follows:
- if flags is just None, return immediately, there is nothing to do.
- the Force flag is honored (full metrics computation) and replaced
with ForceDraw.
- the FitCursor flag is honored and removed from the flags.
- the SinglePar update is added if ForceDraw is not in flags and only
the current par has been modified.
The remaining flags are only then added to the BufferView update
flags, and the update strategy is computed for the next paint event.
Finally the dubious call to updateMacros in updateMetrics has been
removed for performance reasons.
Now painting the workarea is done at paint events as should be.
Explicit painting after updating metrics has been replaced by a much
lighter procedure (updatePosCache) to update the insets positions cache.
Expected benefits:
- better performance
- proper use of subpixel aliasing
The LyXRC variable use_qimage is not needed anymore and is therefore removed.
This patch makes sure that, every time a ParagraphMetrics has its
position set, the inset positions for the insets held by this
paragraph are remembered too.
This is complementary to BufferView::updatePosCache, but I do not have
hard evidence that this is required other than to increase robustness.
It may help in some cases when scrolling the document (scrollbar,
cursor up/down, page up/down).
The trick is to remember in BufferView what has been done at the
previous draw, so that the row that contained the caret can be
repainted if needed.
To this end, add an argument paint_caret to BufferView, although
painting the caret is not the job of the BufferView (at this point).
BufferView::needRepaint will act as an interface with
TextMetrics::drawParagraph to know whether the painting of a given
row should be forced.
Currently everything is done at the top row level, so that, if the
caret is in a large table, the whole table will have to be repainted.
It is not clear yet that this is necessary.
Normally the two stages of drawing are
1/ compute metrics of insets/rows/paragraphs/mathrow...
2/ draw the elements and cache their positions
Now the three stages are
1/ metrics
2/ nodraw: do not draw the elements, but cache their position
3/ draw the elements (and store again their position; it does not
seems to hurt performance).
Revive the NullPainter: this replaces the setDrawingEnabled mechanism
with a painter that does nothing. The advantage is that updatePosCache
(renamed from setPosCache) does not need anymore to be invoked from
the frontend.
updatePosCache (the nodraw stage) is called at the end of
BufferView::updateMetrics.
This commit does a bulk fix of incorrect annotations (comments) at the
end of namespaces.
The commit was generated by initially running clang-format, and then
from the diff of the result extracting the hunks corresponding to
fixes of namespace comments. The changes being applied and all the
results have been manually reviewed. The source code successfully
builds on macOS.
Further details on the steps below, in case they're of interest to
someone else in the future.
1. Checkout a fresh and up to date version of src/
git pull && git checkout -- src && git status src
2. Ensure there's a suitable .clang-format in place, i.e. with options
to fix the comment at the end of namespaces, including:
FixNamespaceComments: true
SpacesBeforeTrailingComments: 1
and that clang-format is >= 5.0.0, by doing e.g.:
clang-format -dump-config | grep Comments:
clang-format --version
3. Apply clang-format to the source:
clang-format -i $(find src -name "*.cpp" -or -name "*.h")
4. Create and filter out hunks related to fixing the namespace
git diff -U0 src > tmp.patch
grepdiff '^} // namespace' --output-matching=hunk tmp.patch > fix_namespace.patch
5. Filter out hunks corresponding to simple fixes into to a separate patch:
pcregrep -M -e '^diff[^\n]+\nindex[^\n]+\n--- [^\n]+\n\+\+\+ [^\n]+\n' \
-e '^@@ -[0-9]+ \+[0-9]+ @@[^\n]*\n-\}[^\n]*\n\+\}[^\n]*\n' \
fix_namespace.patch > fix_namespace_simple.patch
6. Manually review the simple patch and then apply it, after first
restoring the source.
git checkout -- src
patch -p1 < fix_namespace_simple.path
7. Manually review the (simple) changes and then stage the changes
git diff src
git add src
8. Again apply clang-format and filter out hunks related to any
remaining fixes to the namespace, this time filter with more
context. There will be fewer hunks as all the simple cases have
already been handled:
clang-format -i $(find src -name "*.cpp" -or -name "*.h")
git diff src > tmp.patch
grepdiff '^} // namespace' --output-matching=hunk tmp.patch > fix_namespace2.patch
9. Manually review/edit the resulting patch file to remove hunks for files
which need to be dealt with manually, noting the file names and
line numbers. Then restore files to as before applying clang-format
and apply the patch:
git checkout src
patch -p1 < fix_namespace2.patch
10. Manually fix the files noted in the previous step. Stage files,
review changes and commit.
The test added at 359aef92 was incorrect. Actually, Inset::editXY
returns the inset when it is edited and also when it is not, which
can be confusing. So if we are unlucky and the slices positions in the
text area and in the inner inset match, the code below triggers and
many bad things can happen.
Also improve the documentation of Inset::editXY and TextMetrics::editXY.
Fixes#10691.
The test added at 359aef92 was incorrect. Actually, Inset::editXY
returns the inset when it is edited and also when it is not, which
can be confusing. So if we are unlucky and the slices positions in the
text area and in the inner inset match, the code below triggers and
many bad things can happen.
editXY() should probably have a documented way to indicate whether the
cursor has entered the inset, although the test used right now is
probably OK. The inset value is useful mainly for context menus or
tasks that need to know which inset the (x, y) coordinates
point to.
Note finally that the documentation of TextMetrics::editXY is
incorrect concerning the return value. It will return a non-null value
if the cursor is on a not editable inset.
Fixes#10691.
When an inset wants to use the whole available width, it is necessary
to take into account that some of this width may already be taken by
the end-of-paragraph (pilcrow) marker.
Fixes bug #10537.
This is done by two things:
1/ the equation returns LefAlign as display() value
2/ Inset::indent() return a value (in general 0) that should be added
on the left (or right in rtl) of the inset when it is flushed.
The code that uses these values is in TextMetrics::computeRowMetrics.
Use Length instead of HSpace for math_indentation and rename it to mathindent.
Get rid of the string version.
Rename (g|s)etMathIndentation() to (g|s)etMathIndent().
Remove the HSpace class altogether.
Some cleanups to parindent support to look like mathindent.
Instead of using complicated (and wrong) code, it is better to use
getPosNearX here.
Also simplify the code by removing temporary variables.
Fixes part of #10569.
The code that looks whether neighbor block has a different direction
should look to the left or to the right depending on the direction of
the current block.
Fixes part of bug #10569
This allows to somewhat simplify the text and avoid some uses of
Paragraph (in the long term, RowPainter should not have to access
these things).
At the same time do a small cleanup to RowPainter: rename
text_metrics_ to tm_, remove pm_ and width_.
When using Qt stuff in breakAt, it may happen that the row is broken
after an hyphen (whereas the old code would only consider spaces).
The fact that we abuse the Row::right_boundary() property to detect when
a row should be flushed broke justification when a row is cut at an
hyphen.
Fix this by introducing a new Row::flushed() property and set it as needed.
The code that tries to decide whether it is worth splitting a given
text row element had a shortcoming: it did not take into account the
left margin of the new row that would be created.
The problem is that this left margin is not the same as the left
margin of the current row, because there can be for example
indentation effects.
To fix this problem, we pass the amount of available space on the
next row as a parameter of Row::shortenIfNeeded.
Note that there is no need to care about RtL row elements at this
point, since the bidi algorithm will be applied to the row
subsequently.
This avoids some duplicate code. Note that the return value of
Paragraph::getAlign had to be changed. I suspect it was set to char to
avoid reading one header file in Paragraph.h.
1) Distinguish expanding characters from separators, to fit with Qt's notion of
expanding character which comes from the Unicode std. CountExpanders() is moved
to FontMetrics to fix a discrepancy with the duplicate implementation from
598f7e4a.
2) Make these expanders stretch on-screen proportionally to the em of the font.
If a row mixes large and small text, LyX let us see which spaces are set in the
bigger font.
3) Now that the stretch is defined in ems, add a limit such that an expander
never stretches more than 1.5em to avoid weird and hard to read justified lines.
4) Add a return boolean to setSeparatorExtraWidth for future use.
In redoParagraph, this should be done before coping with the insets,
other wise some graphic gliches may occur. This is a better fix for
Fixes bug #10163.
This code is signalled as a copy and paste error, but it is a false
positive.
According to the documentation, adding a comment starting with
// coverity[name_of_error]
should be enough to flag the false positive.
The new functions parBottomSpaging and parTopSapcing return below/above each paragraph.
This allows to remove the TopBottomSpace argument and makes the code a
bit clearer.
Now that Row has a pit() member, it is possible to use it instead of passing an extra pit_type parameter to a function which already has access to a Row.
It is actually easier to set the 20 pixels margin in redoParagraph, since it is not necessary to take newlines in account when deciding what is the real last row of the paragraph.
Moreover this solves the following bug (present in 2.1.x too): when the document ends with a newling, the bottom margin disappears.
Update PAINTING_ANALYSIS with new tasks.
The initial values for maxasc and maxdes (renamed from maxdesc) is obtained as a maximum of max ascents/descents of all row elements.
This allows to get rid of Paragraph::highestFontInRange and FontList::highestInRange.
Some auxilliary variables declarations are also moved to where they are needed.
Let breakRow return a boolean indicating whether an additional row is
required (after a newline) and use that to replace the code that added
an extra row when a paragraph ends with a newline.
There are two main cases:
* when drawing is disabled from the start, use a simplified code that only paints insets (in order to cache positions).
* when the row is not visible, do the same.
The goal of this optimization is to be able to always run a no-drawing draw after the metrics have been computed.
