<html> <head> <base href="https://bugs.freedesktop.org/" /> </head> <body> <div> <a class="bz_bug_link bz_status_RESOLVED bz_closed" title="RESOLVED FIXED - [PATCH] Add support for TextMarkup Annotations in glib frontend" href="https://bugs.freedesktop.org/show_bug.cgi?id=51487#c49">Comment # 49</a> on <a class="bz_bug_link bz_status_RESOLVED bz_closed" title="RESOLVED FIXED - [PATCH] Add support for TextMarkup Annotations in glib frontend" href="https://bugs.freedesktop.org/show_bug.cgi?id=51487">bug 51487</a> from <a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> Carlos Garcia Campos</a> <pre>(In reply to <a href="show_bug.cgi?id=51487#c48">comment #48</a>) > What's the problem with using get_selection_region? The only problem is that it's deprecated. > I was even thinking on > adding a poppler_markup_annot_set_to_selection (annot, rectangle) that would > internally > add the quadrilaterals according to the selection given by the rectangle, > using a method similar to get_selection_region inside... This would save us > from all the Quads->PopplerQuads->EvQuads conversions while constructing the > annotation during drag and drop. text markup annotations already take a GArray as constructor parameter. > Otherwise, I believe that poppler_page_get_selection_region is exactly the > method we should use. I even argue for having a method > poppler_page_get_quadrilaterals_from_region or something like that that > already constructs the array of quadrilaterals... Having a new dedicated method for this sounds good to me. I would use something like GArray *poppler_page_get_quadrilaterals_for_area (PopplerPage *page, PopplerRectangle *area); because from_region() sounds to me like if the function would receive a cairo_region_t as parameter. We should also document that this doesn't return lines, but blocks. > The fact that the rectangles associated to lines overlap is what makes the > cairo_region_t method useless (cairo_region_t is optimized for clipping and > drawing, so when you make a union_rectangle, it basically merges rectangles > when it can). I think that is is fairly trivial to make the rectangles not > to overlap, we need to override TextSelectionSizer::visitLine. > > Currently it does something like this > > margin = (line->yMax - line->yMin) / 8; > x1 = line->edge[edge_begin]; > y1 = line->yMin - margin; > x2 = line->edge[edge_end]; > y2 = line->yMax + margin; > > rect = new PDFRectangle (floor (x1 * scale), > floor (y1 * scale), > ceil (x2 * scale), > ceil (y2 * scale)); > so you clearly see that it's adding a margin to the rectangles. We could > just override the method in a new class and use this class in the > poppler_page_quadrilaterals_from_selection or in the > poppler_anno_set_quadrilaterals_from_selection I think we still want some margin for some of the text markup annotation, at least for highlight, no? TextSelectionSizer is only used by getSelectionRegion. Instead of adding a new derived class, we can just add another construct parameter, passed also to getSelectionRegion, to decide whether to use a margin or not.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>