Re: Update - LaTeX markup project

[Michael Goffioul]

On Sat, Jul 13, 2013 at 11:10 AM, Patrick Noffke <address@hidden> wrote:

Andrej,

On Thu, Jul 11, 2013 at 3:18 PM, Andrej Lojdl <address@hidden> wrote:
> So after quite time exploring how classes are related, I have small
> proposal. New abstract class should be made on top of ft-renderer and
> latex_renderer. It should be placed in txt-eng.h . It should have only two
> attributes pixels and bbox. And inside opengl_renderer , text_procesor
> should be define as our_abstract_class text_renderer . ft_renderer is
> already defined, so latex_renderer should be defined in separate file. I'm
> not sure in what moment we should pik one of these two renderers, based on
> interpreter value?
>

Basically, I think you are understanding the idea.  But there a few
details that I want to go over.

1. I gave you wrong information before (sorry about that).  I said
only glps_renderer was ever instantiated.  I used grep to try and find
it or opengl_renderer, and I don't know what I found, but that was
wrong.  In __init_fltk.cc, opengl_renderer is a private member
variable of OpenGL_fltk.  As Michael said, this is used for on-sreen
mode.  In OpenGL_fltk::draw, glps_renderer is instantiated as a local
variable if rendering in print mode (print_mode == true).

2. The text class has a 'renderer' member variable, which is currently
of type ft_render.  This only exists if HAVE_FREETYPE is #define'd.
And within graphics.cc, there is more code that is #define'd out if
HAVE_FREETYPE is not set.  As far as I can tell, you get no text if
HAVE_FREETYPE is not defined.  Question for the list -- would it make
sense to require both FLTK and FreeType in order to enable FLTK
rendering (and similarly, for QtHandles, we would require FreeType in
order to use Qt)?

3. The availablity of ft_render is decided at compile-time, whereas I
think it makes sense to decide latex_renderer availability at run-time
(i.e. if the required external programs are found in the path).  Then
when you set the 'interpreter' property, you will have to select
between ft_render or latex_renderer.  We could have the text class
contain (as a member variable) a pointer to the proposed base
text_renderer class, which defaults to an instance of ft_render (if
available).  But if both HAVE_FREETYPE is not defined and the latex
binaries are not available, this will be a nullptr.  Then we would
need to add nullptr checks around all uses of text_renderer within the
text class.  Perhaps this is the way to go.  Are there other
suggestions?

4. It appears the the font and the color affect the state of the
ft_render class (in text::properties::update_font).  Should these
attributes affect the latex rendering, or would we expect the font and
color to be specified in a latex string?

5. When is the interpreter property set?  Is it before setting other
properties like the font and color?  If not, I could see how the font
and color might not get propogated to ft_render (if latex was
previously set as the interpreter).  Maybe it makes sense to always
pass this down to the ft_render instance, if it exists, regardless of
what interpreter is set.  Or is there a way to ensure interpreter is
always set first?

I believe we should go another way. IMO having a ft_render (or any text renderer object) as a member of the text object is more of a problem than anything, as well as keeping stateful information in it (like font and color), for what I think is a little gain.

What about simply instantiating the text rendering object only when we need it, render the text, then discard the text renderer object? In practice, I'm thinking about something along these lines (this is only conceptual code):

class base_text_renderer

{

  ...

};

class ft_text_renderer : public base_text_renderer

{

  ...

};

class latex_text_renderer : public base_text_renderer

{

  ...

};

class text_renderer

{

public:

  text_renderer (const caseless_str& interpreter = caseless_str ())

    : rep (0)

    {

      if (interpreter.compare ("latex"))

        rep = new latex_text_renderer ();

      else

        rep = new ft_text_renderer ();

    }

  ~text_renderer (void) { delete rep; }

  void set_font (...) { rep->set_font (...); }

  void set_color (...) { rep->set_color (...); }

  void text_to_pixel (...) { rep->text_to_pixel (...); }

  ...

private:

  base_text_renderer *rep;

};

void

text::properties::update_text_extent (void)

{

  ...

  text_renderer r (get_interpreter ());

  r.set _font (...);

  r.set_color (...);

  r.text_to_pixel (...);

  ...

}

The above code is not highly efficient, because it re-renders the text on every update_text_extent call (that's also the case with the existing code). In future optimization, we could render the text only when we actually need it instead, and cache the result (which would be invalidated by update_text_extent). But this can be done later.

Michael.