Le 21/04/2013 21:24, pdv a écrit :
Hereafter I've listed some comments which might be helpful or answer
suggestions you've made earlier. (btw I couldn't find support/pmprof.h)
If you have comments, questions ... I'm looking forward to hearing from
you.

I need some time to really understand the code. I will probably do that during LDM in Milano at the beginning of May. In the meantime, here are some remarks.

Some general comments:

- please drop the //pdv comments, since they will eventually not be necessary anymore. - please add the #include directives at the right place (separate Qt headers from support and frontend ones).

- there are 4 functions where the textwidth must be calculated: in
GuiPainter::text() and in 3 functions in the TextMetrics class, in order
of complexity:
(1) cursorX(): find the position in pixels, given the cursor position in
characters,
(2) rowBreakPoint(): find the next breakpoint of a row, this will always
break between words,
(3) getColumnNearX(): actually the reverse operation of (1); more
complex than (2) since the position can also be located within a word.

For the time being these 3 functions are still independent, although
they are somewhat similar and maybe some more streamlining is possible
here. I'm also aware that the code for getColumnNearX() is rather
complex.

I would think that all computation could be done in rowBreakPoint and that information could be kept in some data structure, so that the other methods can reuse them.

The widths calculated are cached in a std::map<docstring, int>. I've
also tried QHash but since docstring has no qhash function all strings
must then be converted to QStrings and there is no speed gain.

OK.

I use only one map, the fonts are coded as a string of 4 characters
(family, series, shape, size) which is then used as a prefix for the
key. I have not tried alternatives like using a map for each used font.

Why do you add 0x61 to the values?

The map itself is stored in the BufferView class; In this way there is
one map for each document; when storing the map in the TextMetrics
class, multiple maps are generated. I have only tested this with simple
documents (no child documents Š).

I do not think there is a need to have a map per document. A shared map stored in TextLetrics should be OK (like singleWidth currently).

When typing it's unavoidable to generate all partials of a word; these
are removed again from the map so that only the final word remains;
However nothing is done to remedy the reverse: when deleting a word
character by character all partials will end-up in the map;

Did you do some measurement to ensure that there is a gain of doing that?

The old code is still in place and the old painting character by
character can be restored by changing 2 appropriate macro definitions.

Thanks,
JMarc


Reply via email to