GNU bug report logs - #37755
Logic in init_fringe_bitmap should be moved to backends (maybe rif->define_fringe_bitmap)

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37755 in the body.
You can then email your comments to 37755 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Logic in init_fringe_bitmap should be moved to backends (maybe
 rif->define_fringe_bitmap)
Date: Mon, 14 Oct 2019 23:30:08 -0300

In fringe.c you have init_fringe_bitmap with this structure:

```
#if defined (HAVE_X_WINDOWS)
...
#ifdef USE_CAIRO
...
#endif
...
#endif
#ifdef HAVE_NTGUI
...
#endif
  if (!once_p)
    {
    ....
    rif->define_fringe_bitmap (...)
    ....
    }
```

Now, this is backend specific code that should be moved behind the
redisplay_interface. It seems to me that the obvious candidate to host
that code is define_fringe_bitmap. The once_p clause is related to
pdumper which I ignore all about.

Best regards
--
Carlos

Message #8 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to backends
 (maybe rif->define_fringe_bitmap)
Date: Tue, 15 Oct 2019 12:33:31 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Mon, 14 Oct 2019 23:30:08 -0300
> 
> In fringe.c you have init_fringe_bitmap with this structure:
> 
> ```
> #if defined (HAVE_X_WINDOWS)
> ...
> #ifdef USE_CAIRO
> ...
> #endif
> ...
> #endif
> #ifdef HAVE_NTGUI
> ...
> #endif
>   if (!once_p)
>     {
>     ....
>     rif->define_fringe_bitmap (...)
>     ....
>     }
> ```
> 
> Now, this is backend specific code that should be moved behind the
> redisplay_interface.

Yes, it should.

> It seems to me that the obvious candidate to host that code is
> define_fringe_bitmap.

No, I think we need another interface, as define_fringe_bitmap is used
directly from other places.

Message #11 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Tue, 15 Oct 2019 16:04:09 -0300

(Sorry, forgot to CC debbugs, resending all together below)

Hi Eli,

I've sketched the tree of define/init calls below and I believe I
pretty much understand how the initialization sequence works now and
I'm ready to think about how to properly extend the redisplay
interface in order to include init_fringe_bitmap.

But could you check that my understanding of the flow is sound?


# On emacs startup initialize all standard bitmaps but
# postponing rif->define_fringe_bitmap until frame creation

init_fringe_once
    init_fringe_once_for_pdumper
        for each standard bitmap
            init_fringe_bitmap (oncep = 1)
                since oncep: do not try to destroy previous
                since !rif: do not rif->define_fringe_bitmap

# When a frame is created, actually call rif->define_fringe_bitmap
# for each standard bitmap and also for lisp defined
# bitmaps that were created in a daemon with no frame / rif

gui_init_fringe (I assume this is called once per frame)
    for each standard bitmap
        rif->define_fringe_bitmap
    for each additional bitmap (recently introduced by [1])
        rif->define_fringe_bitmap

# When defined from lisp do both steps at once (init and rif->define)
# except that we're in daemon with no frame / rif

define-fringe-bitmap (lisp)
   init_fringe_bitmap (oncep = 0)
      when rif && !oncep:
           since !oncep: do destroy previous (if any)
           since rif: do rif->define_fringe_bitmap (if not in daemon)

Also, I understand the roles of init_fringe_bitmap and define_fringe_bitmap as:

init_fringe_bitmap
   Initialize bitmap as far as possible without assuming there is a rif/frame
   available. For example, bit shuffling, endianness stuff.

define_fringe_bitmap
   Do the remaining, backend specific, initialization, which is now possible
   because we have a rif/frame. For example, create Cairo surfaces.

Now I see a problem here: abstracting from the platform/backend by
encapsulating platform/backend related logic in the rif is only
possible when there is a rif at hand! And there is no rif when there
is no frame. So that's probably the reason why quite a few
platform-specific assumptions have leaked into fringe.c. The sad part
is how coupled are those bit manipulations to those backends, it's not
like there is some platform-specific part first and then a
backend-specific part, because both are too intermingled and they
logically belong together.

So I'm again tempted to move init into define, I don't think there is
any real gain in splitting the initialization this way, it's not like
the initial part is time consuming or resource intensive so we better
do it ASAP or whatever and, as I've said, it neither provides an
orthogonal abstraction, there is no cartesian product of platforms and
backends here, so to speak. And part of the complexity of the
initialization sequence above is due to this split.

What do you think?

I'll be sending a patch quite soon.

---

[1] [PATCH] Fix initialization of user-defined fringe bitmaps in daemon

On Tue, Oct 15, 2019 at 6:33 AM Eli Zaretskii <eliz <at> gnu.org> wrote:
>
> > From: Carlos Pita <carlosjosepita <at> gmail.com>
> > Date: Mon, 14 Oct 2019 23:30:08 -0300
> >
> > In fringe.c you have init_fringe_bitmap with this structure:
> >
> > ```
> > #if defined (HAVE_X_WINDOWS)
> > ...
> > #ifdef USE_CAIRO
> > ...
> > #endif
> > ...
> > #endif
> > #ifdef HAVE_NTGUI
> > ...
> > #endif
> >   if (!once_p)
> >     {
> >     ....
> >     rif->define_fringe_bitmap (...)
> >     ....
> >     }
> > ```
> >
> > Now, this is backend specific code that should be moved behind the
> > redisplay_interface.
>
> Yes, it should.
>
> > It seems to me that the obvious candidate to host that code is
> > define_fringe_bitmap.
>
> No, I think we need another interface, as define_fringe_bitmap is used
> directly from other places.

Message #14 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Tue, 15 Oct 2019 16:45:20 -0300

[Message part 1 (text/plain, inline)]

This is the modified initialization logic implemented by the attached patch:

# When a frame is created, actually call rif->define_fringe_bitmap
# for each standard bitmap and also for lisp defined
# bitmaps that were created in a daemon with no frame / rif

gui_init_fringe (I assume this is called once per frame)
    for each standard bitmap
        rif->define_fringe_bitmap
    for each additional bitmap (recently introduced by [1])
        rif->define_fringe_bitmap

# When defined from lisp call rif->define_fringe_bitmap
# except that we're in daemon with no frame / rif

define-fringe-bitmap (lisp)
      when rif: rif->define_fringe_bitmap

Much simpler, don't you think? Also removed ~30 LOCs.

Except that I'm overlooking something obvious it seems to do exactly
the same, only that the bit shuffling part is postponed until the
frame is actually created (that is, moved into
rif->define_fringe_bitmap).

Some remarks:

1. There was this comment on top of init_fringe_bitmap: "On MAC with
big-endian CPU, we need to byte-swap each short". But all the code
there were either specific to NTGUI, specific to XWINDOWS or related
to bitmap destruction. So nothing remained that could be actually
moved into the NS backend.

2. I left the HAVE_NTGUI guard even after moving the relevant code
into w32term.c because I'm not sure whether all w32 code share the
NTGUI path or not.

3. The previous implementation modified bits as a way of connecting
init with define, this was the state passed from one stage to the
other. Now, since both parts are done together, there is no need to
modify the passed bits array, the array is only required in order to
initialize backend-specific structures. Nevertheless, I decided to
keep that as it was in order to prevent regressions, but I believe an
implementation that preserves the original value of bits would be
preferable now that it is indeed possible. In particular, for X the
only "backend-specific structure" is an utterly modified bits array
(in some cases shorts are converted into chars, that's why I said in
#37689 that it's difficult for the upper layer to "split rows" after
this kind of manipulations had already taken place).

Any reviews would be much appreciated.

Best regards
--
Carlos

[0001-Fringe-refactor-move-platform-specific-code-into-red.patch (text/x-patch, attachment)]

Message #19 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Wed, 16 Oct 2019 17:18:16 -0300

[Message part 1 (text/plain, inline)]

* Improved commit message according to CONTRIBUTE rules.

* Made some minor changes to avoid conditional compilation warnings.

* I'm aware of some issues when rendering fringe bitmaps (nothing
fatal, though) and I'm still debugging these changes, but it would be
great if you could review them meanwhile.

[0001-Fringe-refactor-move-platform-specific-code-into-rif.patch (text/x-patch, attachment)]

Message #22 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Wed, 16 Oct 2019 19:07:26 -0300

[Message part 1 (text/plain, inline)]

Ok, I found the bug. The init part was in general iterating along bits
by advancing the pointer (i.e. bits++). When I juxtaposed this part to
the define part, that also used bits, then bits was pointing to one
past the end of the bitmap and not to the bitmap itself, as expected
by the define code. Once diagnosed, it was an easy fix. In detail:

1. For Cairo, I followed my own suggestion above of not modifying bits
at all. Both the previous init and define loops are now merged into a
single loop that directly writes to the surface data, instead of
modifying bits.
2. For X11, bits is the only state propagated between init/define and
draw and there was no specific define part before my changes, so the
new define is simply the old init. Here bits is obviously modified in
order to prepare it for the drawing routine.
3. For Win32, I replaced bits++ with bits[i]. Bits is also modified,
although here it's easy to avoid that if so desired because it's only
locally needed to initialize the platform-specific bitmap structure
then used by the drawing routine.
4. For MacOS/NS, there is nothing to be done since there was no
specific init part, as I observed a few posts above.

I've cursorily tested this patch both alone and merged with my "hidpi
fringe" patches and seems to be working fine for the Cairo backend.

The new change amounts to yet another code simplification and an
overall ~40 reduction in LOCs.

Best regards
--
Carlos

[0001-Fringe-refactor-move-platform-specific-code-into-rif.patch (text/x-patch, attachment)]

Message #25 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sun, 20 Oct 2019 15:21:37 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Wed, 16 Oct 2019 19:07:26 -0300
> Cc: 37755 <at> debbugs.gnu.org
> 
> The new change amounts to yet another code simplification and an
> overall ~40 reduction in LOCs.

Sorry, I think I'm missing something here.  It looks like you removed
the call to init_fringe_bitmap during dumping, and left its equivalent
only in define-fringe-bitmap, is that right?  If so, I cannot see how
this could work, because Emacs needs to have the standard fringe
bitmaps (for line truncation, continuation, etc.) be defined even
without a call to define-fringe-bitmap.  If you start the current
master under a debugger after putting a breakpoint in
Fdefine_fringe_bitmap, the breakpoint doesn't break, and yet the
bitmaps are known and will be displayed as needed.

What did I miss?

Message #28 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sun, 20 Oct 2019 12:47:13 -0300

Hi Eli,

thank you for the review.

>It looks like you removed
> the call to init_fringe_bitmap during dumping, and left its equivalent
> only in define-fringe-bitmap, is that right? >

> What did I miss?

The call to gui_init_fringe I guess. Also, notice that
define_fringe_bitmap is quite different than Fdefine_fringe_bitmap.

I suggest you take a look at the modified pseudo code I posted quite a
few message above.

> Emacs needs to have the standard fringe
> bitmaps (for line truncation, continuation, etc.) be defined even
> without a call to define-fringe-bitmap.

This is indeed the case after applying the patch. Some bit shuffling
has been postponed from init_fringe_once to gui_init_fringe, but
that's all.

Now, regarding the dumping stuff you mention, TBH I'm completely
ignorant. So maybe this innocent looking delay of bit shuffling could
have some effect, I don't now, but it's a very different thing from
not initializing standard bitmaps until define-fringe-bitmap is first
called from elisp world.

Besides, whatever is missing after the C static initialization part is
just this *platform dependent* bit shuffling, which I seriously doubt
emacs could make sense of without the appropriate rif at hand, so
quite late in the initialization sequence. I even suggested to avoid
this destructive manipulation of platform independent bitmaps from the
part of the rifs, although I've only followed my suggestion in the
case of cairo, which was quite natural and convenient.

Best regards
--
Carlos

Message #31 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sun, 20 Oct 2019 19:07:11 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Sun, 20 Oct 2019 12:47:13 -0300
> Cc: 37755 <at> debbugs.gnu.org
> 
> > What did I miss?
> 
> The call to gui_init_fringe I guess.

I don't see that call in the patch, nor any changes in gui_init_fringe
that would modify its current effect.

If you mean the existing calls, then they are only made at run time,
which would mean Emacs is dumped without the standard bitmaps?  Why is
that?

> Also, notice that define_fringe_bitmap is quite different than
> Fdefine_fringe_bitmap.

Sure, but I said define-fringe-bitmap, which is the Lisp name of
Fdefine_fringe_bitmap.

> I suggest you take a look at the modified pseudo code I posted quite a
> few message above.

I will, but I'd like to see the full patch as well.

> Besides, whatever is missing after the C static initialization part is
> just this *platform dependent* bit shuffling, which I seriously doubt
> emacs could make sense of without the appropriate rif at hand, so
> quite late in the initialization sequence. I even suggested to avoid
> this destructive manipulation of platform independent bitmaps from the
> part of the rifs, although I've only followed my suggestion in the
> case of cairo, which was quite natural and convenient.

If RIF is the problem, we could make each terminal backend do this
initialization unconditionally at dump time.

Message #34 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sun, 20 Oct 2019 13:32:36 -0300

> I don't see that call in the patch, nor any changes in gui_init_fringe
> that would modify its current effect.

Because nothing changed in gui_init_fringe itself. It did and does:

  for (bt = NO_FRINGE_BITMAP + 1; bt < MAX_STANDARD_FRINGE_BITMAPS; bt++)
      rif->define_fringe_bitmap (bt, fb->bits, fb->height, fb->width);
  for ( ; bt < max_used_fringe_bitmap; bt++)
    rif->define_fringe_bitmap (bt, fb->bits, fb->height, fb->width);

The change affects rif->define_fringe_bitmap instead. It now does:

- Create platform-dependent structures from platform-independent bitmaps.

Previously this was divided between init and define as:

- Init: manipulate platform-independent bitmaps in a platform-dependent way.
- Define: use this platform-dependently shuffled bitmaps to create
platform-dependent structures.

So the only thing that have moved down the initialization sequence is
the bit-shuffling gymnastics which, if any, are done in
gui_init_fringe now.

> Sure, but I said define-fringe-bitmap, which is the Lisp name of
> Fdefine_fringe_bitmap.

I meant to remark that they do quite different things not that you
mistake one for the other, sorry if I wasn't clear.

> > I suggest you take a look at the modified pseudo code I posted quite a
> > few message above.
>
> I will, but I'd like to see the full patch as well.

You have already seen it :)

> If RIF is the problem, we could make each terminal backend do this
> initialization unconditionally at dump time.

According to my rationale above, I don't see any problem at all. But,
as I have said, I ignore everything about the dumper. Yet, I find it
hard to believe that whatever this  dumper thing is, it needs the bits
to be in little-endian, 8-bit per row format, or any other
rif-specific pattern.

Hope it's clearer now.

Message #37 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sat, 26 Oct 2019 13:39:27 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Sun, 20 Oct 2019 13:32:36 -0300
> Cc: 37755 <at> debbugs.gnu.org
> 
> > If RIF is the problem, we could make each terminal backend do this
> > initialization unconditionally at dump time.
> 
> According to my rationale above, I don't see any problem at all. But,
> as I have said, I ignore everything about the dumper. Yet, I find it
> hard to believe that whatever this  dumper thing is, it needs the bits
> to be in little-endian, 8-bit per row format, or any other
> rif-specific pattern.

Sorry, we cannot just ignore the dumping issue.  We don't want to
waste CPU cycles each startup to regenerate these standard bitmaps.
So the fringe bit patterns need to be initialized at dump time and
dumped together with all the other stuff we prepare at that time.

If I understand correctly, the difficulty you had with doing this at
dump time was that frame's RIF was not yet set (because dumping works
in batch mode, where redisplay interface is not set to the correct GUI
frame type).  If so, my suggestion is to call the window-system
specific initialization function directly.  For example, in the X
build, you can add code to syms_of_xterm code to call
x_define_fringe_bitmap, and similarly for Cairo, w32, etc.

Does this proposal resolve the difficulty?  If not, please point out
what else is missing.

Thanks.

Message #40 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sat, 26 Oct 2019 12:46:08 -0300

[Message part 1 (text/plain, inline)]

>
>
> Sorry, we cannot just ignore the dumping issue.  We don't want to
> waste CPU cycles each startup to regenerate these standard bitmaps.
> So the fringe bit patterns need to be initialized at dump time and
> dumped together with all the other stuff we prepare at that time.
>

Ok, I'm not sure about how dumping works, but the patterns that you store
are backend specific. Does the dumper take that into account?

>
>
> Does this proposal resolve the difficulty?  If not, please point out
> what else is missing.
>

I just thought that the change decoupled and simplified the code, but I can
always add an initialization step to the rif, previous to the definition of
the actual bitmaps used by the backend.

[Message part 2 (text/html, inline)]

Message #43 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Carlos Pita <carlosjosepita <at> gmail.com>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sat, 26 Oct 2019 19:03:24 +0300

> From: Carlos Pita <carlosjosepita <at> gmail.com>
> Date: Sat, 26 Oct 2019 12:46:08 -0300
> Cc: 37755 <at> debbugs.gnu.org
> 
>  Sorry, we cannot just ignore the dumping issue.  We don't want to
>  waste CPU cycles each startup to regenerate these standard bitmaps.
>  So the fringe bit patterns need to be initialized at dump time and
>  dumped together with all the other stuff we prepare at that time.
> 
> Ok, I'm not sure about how dumping works, but the patterns that you store are backend specific. Does the
> dumper take that into account?

The pdump file, like the Emacs binary, is architecture-specific.  So
yes, this is inherently taken into account.

>  Does this proposal resolve the difficulty?  If not, please point out
>  what else is missing.
> 
> I just thought that the change decoupled and simplified the code, but I can always add an initialization step to
> the rif, previous to the definition of the actual bitmaps used by the backend.

I don't think you can do that with a RIF, for the reason already
mentioned: Emacs is dumped in batch mode, where we have a frame type
that doesn't support fringes, and doesn't implement the RIF interfaces
you will need.

Message #48 received at 37755-close <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755-close <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sat, 26 Oct 2019 13:11:09 -0300

[Message part 1 (text/plain, inline)]

> I don't think you can do that with a RIF, for the reason already
> mentioned: Emacs is dumped in batch mode, where we have a frame type
> that doesn't support fringes, and doesn't implement the RIF interfaces
> you will need.
>

Ok, I'm not eager to rework this, so if you think that avoiding repeating
that bit shuffling at the beginning is worth the additional coupling and
complexity I trust in your criterion and close this issue. Thanks.

>

[Message part 2 (text/html, inline)]

Message #51 received at 37755 <at> debbugs.gnu.org (full text, mbox):

From: Carlos Pita <carlosjosepita <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 37755 <at> debbugs.gnu.org
Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to
 backends (maybe rif->define_fringe_bitmap)
Date: Sun, 27 Oct 2019 11:47:01 -0300

[Message part 1 (text/plain, inline)]

>
>
>
> > Ok, I'm not sure about how dumping works, but the patterns that you
> store are backend specific. Does the
> > dumper take that into account?
>
> The pdump file, like the Emacs binary, is architecture-specific.  So
> yes, this is inherently taken into account.
>

I kept thinking about this and there is also the fact that is not only the
architecture (I mean x, w32, ns, endianness) that is assumed in that bit
shuffling but also, for example, if we have cairo or pure xlib backend, and
that's because I'm quite sure that code was written with the input format
assumed by the rif (even if the rif still doesn't exist at that point) in
mind. Again, I don't know about the dumper but my intuition says there is
something potentially wrong in this arrangement.

What I proposed is:

1. Static initialization of fringe rif/platform-independent structures,
that I guess will be dumped.

2. Prepare. Per-rif initialization of the rif/platform-dependent
structures. This shouldn't affect the independent structures, although in
some cases the original bit pattern is still destructively changed because
it was simpler to keep the existing implementation.

3. Draw. Rendering of the rif/platform-dependent structures to the screen.

What you argue for is:

1. Idem.

1'. Init. Initialization of the platform-dependent but rif->independent
structures.

2'. Prepare. Initialization of the rif/platform-dependent structures from
the platform-dependent but rif->independent structures.

3. Idem.

Now 1' is just cheap bit shuffling of some twenty or so standard bitmaps
having an average grid of 8x8 bits. Also there are might be bugs if output
patterns are not rif specific and the dumper is unaware of that (again I
can't say for sure because of my lack of knowledge of the dumper).

Moreover, having 1' and 2' merged in 2 may actually speed things up,
because there is no need for two separate iterations over the bitmaps, the
first one producing the bit pattern for the second one. It's natural to
simply iterate over the original pattern and directly produce the input
expected by the particular rendering backend.

So, having exposed my reasoning as detailed as I could, and once again, are
you sure that you want to keep that phase 1' just to save some milli
(micro?) seconds, if any? There is a price in code complexity and the risk
of coupling fringe.c too much with backend specific logic. Also, suppose
that there is a problem with this cairo vs xlib decision hardcoded there,
before the dumping happens. One option is to move that xlib vs cairo
decision to the rif (that is to 2 or 2' above). And this way you will be
converging to an empty 1' and 2'->2.

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.