In article <5176ae47.9050...@lyx.org>, Jean-Marc Lasgouttes <lasgout...@lyx.org> wrote:
> 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. I agree; this was just temporary to easily find what I did modify. > - 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? That's just for easy reading when looking at what exactly gets written to the map; In this way the codes are a, b, c ... > > > 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 S). > > 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). I was worried that the map might grow out of proportion, e.g. when leaving the application open for a long time and since there are more words than characters this will be worse than for singleWidth. > > > 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? I don't think it makes so much difference in time. It was merely to avoid filling the map with useless entries, especially if a single map is used. Thanks for the feedback. > > > 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
