Bug#914153: Update to version 2.3.0-3 breaks Megaglest

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Evgeny Kapun
Package: libftgl2
Version: 2.3.0-3

After updating libftgl2 from version 2.1.3~rc5-5 to version 2.3.0-3, text rendering in Megaglest is broken. Text is shown correctly in menus, but text displayed in the game itself is replaced by white rectangles.

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
Hi Evgeny,

2018-11-19 23:44 Evgeny Kapun:
>Package: libftgl2
>Version: 2.3.0-3
>
>After updating libftgl2 from version 2.1.3~rc5-5 to version 2.3.0-3, text rendering in Megaglest is broken. Text is shown correctly in menus, but text displayed in the game itself is replaced by white rectangles.

Thanks for the report.

Any idea of what's going on, Frank?

For me it works in some menus, there are white squares in others.  zaz,
another application using ftgl, seems to work fine, while critterding
also shows white squares.


Cheers.
--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Frank Heckenbach-4
Hi Evgeny,

> >After updating libftgl2 from version 2.1.3~rc5-5 to version 2.3.0-3, text rendering in Megaglest is broken. Text is shown correctly in menus, but text displayed in the game itself is replaced by white rectangles.
>
> Thanks for the report.
>
> Any idea of what's going on, Frank?
>
> For me it works in some menus, there are white squares in others.  zaz,
> another application using ftgl, seems to work fine, while critterding
> also shows white squares.

That's probably a consequence of a fix made by sammy in 2009(!)
(commit 29603ae: "Remove GL_BLEND tampering. It's the caller
application's responsibility to enable or disable blending").

For some reason, that patch (and a few others) hadn't made it into
Debian before and only now got in as a "bycatch" when we
synchronized everything.

Though it is an incompatible change, it was a necessary one, since
what the library did before was wrong (which prompted a bug report
of my own, #742469 which would solve it for some particular cases
while adding a dependency on GL_GLEXT_PROTOTYPES). So the better
solution is probably the earlier (and now current) one which puts
the responsibility on the caller.

I had seen the same problem in another test program recently, and in
this case it helped to add the following two lines in an appropriate
place. You can try the same in your code (shouldn't hurt if compiled
with older ftgl versions as well):

  glEnable(GL_BLEND);
  glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);

Or, if you like a better blending function (see my description in
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=742469), you can
use this instead:

#define GL_GLEXT_PROTOTYPES

  glEnable(GL_BLEND);
  glBlendFuncSeparate(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA,
                      GL_ONE, GL_ONE_MINUS_SRC_ALPHA);

Regards,
Frank

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
Control: tags -1 + serious

Hi,

Em qua, 21 de nov de 2018 às 02:10, Frank Heckenbach
<[hidden email]> escreveu:

>
> Hi Evgeny,
>
> > >After updating libftgl2 from version 2.1.3~rc5-5 to version 2.3.0-3, text rendering in Megaglest is broken. Text is shown correctly in menus, but text displayed in the game itself is replaced by white rectangles.
> >
> > Thanks for the report.
> >
> > Any idea of what's going on, Frank?
> >
> > For me it works in some menus, there are white squares in others.  zaz,
> > another application using ftgl, seems to work fine, while critterding
> > also shows white squares.
>
> That's probably a consequence of a fix made by sammy in 2009(!)
> (commit 29603ae: "Remove GL_BLEND tampering. It's the caller
> application's responsibility to enable or disable blending").
>
> For some reason, that patch (and a few others) hadn't made it into
> Debian before and only now got in as a "bycatch" when we
> synchronized everything.
>
> Though it is an incompatible change, it was a necessary one, since
> what the library did before was wrong (which prompted a bug report
> of my own, #742469 which would solve it for some particular cases
> while adding a dependency on GL_GLEXT_PROTOTYPES). So the better
> solution is probably the earlier (and now current) one which puts
> the responsibility on the caller.
>
> I had seen the same problem in another test program recently, and in
> this case it helped to add the following two lines in an appropriate
> place. You can try the same in your code (shouldn't hurt if compiled
> with older ftgl versions as well):
>
>   glEnable(GL_BLEND);
>   glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
>
> Or, if you like a better blending function (see my description in
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=742469), you can
> use this instead:
>
> #define GL_GLEXT_PROTOTYPES
>
>   glEnable(GL_BLEND);
>   glBlendFuncSeparate(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA,
>                       GL_ONE, GL_ONE_MINUS_SRC_ALPHA);

I think that we should revert this change for the time being, though,
because if it was working in this way for about a decade and the
programs using FTGL worked "fine" despite having some bug there,
there's no need to change this now and break applications with only a
few weeks to fix this in 15+ other packages before the freeze.

Otherwise we have to get the fix in several of this packages, which is
way more difficult, specially if not well maintained in Debian,
upstream or both.


Cheers.
--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Frank Heckenbach-4
Hi,

> I think that we should revert this change for the time being, though,
> because if it was working in this way for about a decade and the
> programs using FTGL worked "fine" despite having some bug there,

Sorry, they did *NOT* all work fine, see my bug report. And if we
apply my patch from 742469 instead, that might break programs in
other ways (and will require yet another breaking change in the
future, when we apply sammy's patch properly).

> there's no need to change this now and break applications with only a
> few weeks to fix this in 15+ other packages before the freeze.

For me, there was very much a need to change. This was one of the
reasons I started working on ftgl at all if you remember. Without
either fix (sammy's or mine), ftgl is useless to me, so I'm not
gonna do this in my repo.

> Otherwise we have to get the fix in several of this packages, which is
> way more difficult, specially if not well maintained in Debian,
> upstream or both.

As I said, it's a messy situation. A bug that was actually fixed in
2009, not applied downstream, rediscovered by me in 2014, and
accidentally pulled downstream by syncing from the repo you pointed
me do early this year (which I had (naively?) assumed to be mostly
in sync with your version). I'm used to bugs in Debian getting
ignored for years and the wontfixed (or silenly buried), but that
mess seems to be special even in comparison.

So what can we actually do now?

- If you view it as an ABI breaking change, I can bump the version
  to 3.0.0, just let me know. (This would mean that programs using
  the library would need to be rechecked anyway, right? So if we
  document this prominently, they'll know what to look for, both in
  source code and in program behaviour.)

- If you insist on a version without either of those bugfixes, do
  this in your code, but then I'm out, sorry. For me this will mean
  at least 2 more years working with an out-of-distro version (and I
  fear it would get neglected again, so maybe more like 10 more
  years or forever), so I'd have no reason to care about the distro
  version.

- Otherwise just let the other packages find and fix the resulting
  bugs, like the saying goes "better ask for forgiveness than for
  permission".

- A slightly more complex solution would be a flag to select the
  behaviour. Of course, it would need to be a runtime flag. It may
  default to the old behaviour, but that should be deprecated (and
  print a strong depreciation message -- unfortunately that would
  have to be at runtime too AFAICS, or is there a way to warn at
  compile time when a program, say, does *not* reference a
  particular function?).

Cheers,
Frank

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
In reply to this post by Evgeny Kapun
Hi Frank,

Em dom, 25 de nov de 2018 às 18:29, Frank Heckenbach
<[hidden email]> escreveu:

>
> : Perhaps the only sane way out is to add *two* new versions of each
> : rendering function, one with each behaviour, and deprecate the old
> : ones entirely. This will require changes in all applications (if
> : only to select the correct one of the two which should be easy if
> : ones knows which branch of the library they used before), but at
> : least it will be clear at compile time.
>
> That seems to me the best solution indeed. So I can offer this:
>
> - I add these two new versions for all functions involved (quite a
>   few); we'd just need to agree about naming ("old" and "new" won't
>   do, since in this complicated situation it's not even clear which
>   one is old and which one is new; different users will view it
>   differently, just like in special relativity ;), also "old" and
>   "new" in function names always sounds silly; so perhaps something
>   like "RenderBasic" and "RenderDefault" or so ...).
>
> - I deprecate the "Render" functions, adding a special README to
>   explain things.
>
> - I change my sources to use "RenderBasic" (or whatever it'll be
>   called) and test them. Other users of this fork will have to do
>   the same (hopefully after seeing the deprecation warnings and
>   reading that README).
>
> - I release 2.4.0 with those changes.
>
> - You put 2.4.0 in Debian. Applications using it will get the
>   deprecation warnings, but should otherwise work.
>
> - A bit later, I remove the deprecated functions and release 3.0.0.
>
> - After the release of Buster, you put 3.0.0 in Debian with the
>   required transitions.
>
> - Applications using it will break, but fixing them will only
>   require changing "Render" to "RenderDefault" in some places
>   (which are found by compiler errors). This can also be done before
>   you upload 3.0.0 (as "RenderDefault" will be available in 2.4.0
>   already), so the transition can be smoohter.
>
> Does this sound like a viable plan?

I am not sure if I understand.  According to your plan, do the
applications in Debian using ftgl will need to change anything at all
to keep working?  Because there's not much time for making changes
before the freeze, I will be quite busy for a couple of weeks at least
(so please excuse delays in replies if they don't arrive in a timely
manner), and changes of this kind usually take months to be
accomplished.  It's not like we can commit changes to one or several
git repos and rebuild the packages, it's quite more complex than that.

IMO from the Debian side and for Buster there's no material time for
changes to all packages that depend on fgtl, the only practical
solutions are either to revert the change making some applications
unuseable via extra patch from Debian; or have this transition and
deprecation messages etc. in a way that existing packages don't need
changes at all.

Sorry, but I don't think that anything else works for Buster.


Cheers.
--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Frank Heckenbach-4
Hi Manuel,

> > That seems to me the best solution indeed. So I can offer this:
> >
> > - I add these two new versions for all functions involved (quite a
> >   few); we'd just need to agree about naming ("old" and "new" won't
> >   do, since in this complicated situation it's not even clear which
> >   one is old and which one is new; different users will view it
> >   differently, just like in special relativity ;), also "old" and
> >   "new" in function names always sounds silly; so perhaps something
> >   like "RenderBasic" and "RenderDefault" or so ...).
> >
> > - I deprecate the "Render" functions, adding a special README to
> >   explain things.
> >
> > - I change my sources to use "RenderBasic" (or whatever it'll be
> >   called) and test them. Other users of this fork will have to do
> >   the same (hopefully after seeing the deprecation warnings and
> >   reading that README).
> >
> > - I release 2.4.0 with those changes.
> >
> > - You put 2.4.0 in Debian. Applications using it will get the
> >   deprecation warnings, but should otherwise work.
> >
> > - A bit later, I remove the deprecated functions and release 3.0.0.
> >
> > - After the release of Buster, you put 3.0.0 in Debian with the
> >   required transitions.
> >
> > - Applications using it will break, but fixing them will only
> >   require changing "Render" to "RenderDefault" in some places
> >   (which are found by compiler errors). This can also be done before
> >   you upload 3.0.0 (as "RenderDefault" will be available in 2.4.0
> >   already), so the transition can be smoohter.
> >
> > Does this sound like a viable plan?
>
> I am not sure if I understand.  According to your plan, do the
> applications in Debian using ftgl will need to change anything at all
> to keep working?  Because there's not much time for making changes
> before the freeze, I will be quite busy for a couple of weeks at least
> (so please excuse delays in replies if they don't arrive in a timely
> manner), and changes of this kind usually take months to be
> accomplished.  It's not like we can commit changes to one or several
> git repos and rebuild the packages, it's quite more complex than that.
>
> IMO from the Debian side and for Buster there's no material time for
> changes to all packages that depend on fgtl, the only practical
> solutions are either to revert the change making some applications
> unuseable via extra patch from Debian; or have this transition and
> deprecation messages etc. in a way that existing packages don't need
> changes at all.
>
> Sorry, but I don't think that anything else works for Buster.

As I said, in the first step (2.4.0), they should not (assuming some
new warnings while recompiling are no problems).

Some changes will be necessary for 3.0.0, after Buster.

Cheers,
Frank

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
In reply to this post by Evgeny Kapun
Em qua, 6 de fev de 2019 às 04:15, Frank Heckenbach
<[hidden email]> escreveu:

>
> > Em qui, 3 de jan de 2019 às 22:56, Frank Heckenbach
> > <[hidden email]> escreveu:
> > >
> > > According to https://release.debian.org/buster/freeze_policy.html:
> > >
> > > 2019-01-12 - Transition freeze
> > >
> > > Is there still time yet to get a fix in, or is it FUBAR already?
> >
> > Transition freeze means ABI changes in libraries are forbidden.  We're
> > almost in soft-freeze now, more info at:
> > https://lists.debian.org/debian-devel-announce/2019/01/msg00008.html
>
> So do we have time until the soft freeze on 2019-02-12 (I hope not)
> or the full freeze (2019-03-12) to get a 2.4.0 uploaded?

I believe so, yes, maybe even for the soft freeze (I am not sure how
much the time shortens for migrating packages fixing important bugs,
it varies).


> > I have to review the situation again, I completely swapped it out of
> > my memory.
>
> My suggestion of 2018-11-25 still stands. But if you want me to do
> my part of it, please do your review quickly and tell me soon
> (or, if it's indeed necessary for the soft freeze, immediately) to
> avoid running out of time.

Your plan sounds OK.  Changing packages after the release, with time,
should be OK.  I can submit automatic bug reports for the affected
packages.

Maybe it would even be possible to have the applications set a global
variable, e.g.:

  enum class Render { Default = 1, Basic };
  FTGL->setRender(Render  renderType);

and then the Render() function(s) would dispatch to either
RenderDefault() or RenderBasic() versions as appropriate?


Cheers.
--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Frank Heckenbach-4
> > My suggestion of 2018-11-25 still stands. But if you want me to do
> > my part of it, please do your review quickly and tell me soon
> > (or, if it's indeed necessary for the soft freeze, immediately) to
> > avoid running out of time.
>
> Your plan sounds OK.  Changing packages after the release, with time,
> should be OK.  I can submit automatic bug reports for the affected
> packages.

OK.

> Maybe it would even be possible to have the applications set a global
> variable, e.g.:
>
>   enum class Render { Default = 1, Basic };
>   FTGL->setRender(Render  renderType);
>
> and then the Render() function(s) would dispatch to either
> RenderDefault() or RenderBasic() versions as appropriate?

I generally don't much like global flags, but in this particular
case, it might be the least painful approach.

It wouldn't be foolproof. If two pieces of code, e.g. libraries,
that are used in the same program, use Render() with different
settings of this flag, one of them would do the wrong thing. (And
manually changing this flag every time code from the other library
may be used would be a maintenance nightmare.)

So maybe the following is even easier to implement, while also not
foolproof:

- No RenderDefault() and RenderBasic(), just Render() as is.

- A flag similar to your proposal (though I wouldn't actually call
  it "Render..."; if we aren't renaming the Render functions, we can
  use a more specific name), such as LegacyOpenGLState, and it can
  be a bool actually.

- However, ftgl will only allow setting this flag to one value ever
  (but any number of times, so libraries that expect the same
  setting can work together). I.e., if it's set to false, further
  calls setting it to false will succeed, but a call setting it to
  true will abort, indicating a mix of incompatible code pieces; and
  vice versa.

  If it's never set, if will default to true (legacy behaviour);
  I'll have to accept that.

- In my code I'll set it to false; likewise for others relying on
  this behaviour. (My code actually contains libraries using ftgl,
  so I'll make sure to set it from within the libraries, rather than
  the programs using them.)

- Programs relying on the "old" behaviour need no change
  immediately, but should soon (so for Debian, after the release of
  Buster), add a "true" call. This would be a single line change
  near the start, so easy to do even if it affects a number of
  packages.

- Both kinds of program will need to require ftgl>=2.4.0 because of
  the new call.

- Sometime in the future, I hope I can require setting this flag,
  first with a warning, later with an error if it's not set before
  the first rendering. Provided you've added the call to all
  packages by then, nothing will break.

Cheers,
Frank

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
Em qui, 7 de fev de 2019 às 04:32, Frank Heckenbach
<[hidden email]> escreveu:

>
> > > My suggestion of 2018-11-25 still stands. But if you want me to do
> > > my part of it, please do your review quickly and tell me soon
> > > (or, if it's indeed necessary for the soft freeze, immediately) to
> > > avoid running out of time.
> >
> > Your plan sounds OK.  Changing packages after the release, with time,
> > should be OK.  I can submit automatic bug reports for the affected
> > packages.
>
> OK.
>
> > Maybe it would even be possible to have the applications set a global
> > variable, e.g.:
> >
> >   enum class Render { Default = 1, Basic };
> >   FTGL->setRender(Render  renderType);
> >
> > and then the Render() function(s) would dispatch to either
> > RenderDefault() or RenderBasic() versions as appropriate?
>
> I generally don't much like global flags, but in this particular
> case, it might be the least painful approach.
>
> It wouldn't be foolproof. If two pieces of code, e.g. libraries,
> that are used in the same program, use Render() with different
> settings of this flag, one of them would do the wrong thing. (And
> manually changing this flag every time code from the other library
> may be used would be a maintenance nightmare.)
>
> So maybe the following is even easier to implement, while also not
> foolproof:
>
> - No RenderDefault() and RenderBasic(), just Render() as is.
>
> - A flag similar to your proposal (though I wouldn't actually call
>   it "Render..."; if we aren't renaming the Render functions, we can
>   use a more specific name), such as LegacyOpenGLState, and it can
>   be a bool actually.

OK, sounds good.  Your name is better, mine it was only an example.

The idea of using both RenderDefault() and RenderBasic() as well as
the flag, would equally work if you have just Render() and the
behaviour of one render or the other nested in an "if/else" based on
the flag.  Whatever makes more sense to you.  I suggested it because
in that way it is easier to change or patch the packages.

OK to the rest of the message.

Let me know when it's ready, I'll try to upload the new version within
the same day that you have the new release ready.


Cheers.
--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Frank Heckenbach-4
> The idea of using both RenderDefault() and RenderBasic() as well as
> the flag, would equally work if you have just Render() and the
> behaviour of one render or the other nested in an "if/else" based on
> the flag.  Whatever makes more sense to you.  I suggested it because
> in that way it is easier to change or patch the packages.

Creating the two versions would be a lot more work, and in the end
packages need to be patched anyway.

> Let me know when it's ready, I'll try to upload the new version within
> the same day that you have the new release ready.

I did it, and briefly tested it with my code.

I think you can upload it and other packages should see no change in
behaviour for now.

But please remember (see ~/work/ftgl/README-LegacyOpenGLState),
after the release of Buster, add

  FTLibrary::Instance().LegacyOpenGLState(true);

and depend on ftgl>=2.4.0 in all packages that use FTGL.

Cheers,
Frank

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
Em qui, 7 de fev de 2019 às 22:47, Frank Heckenbach
<[hidden email]> escreveu:

>
> > The idea of using both RenderDefault() and RenderBasic() as well as
> > the flag, would equally work if you have just Render() and the
> > behaviour of one render or the other nested in an "if/else" based on
> > the flag.  Whatever makes more sense to you.  I suggested it because
> > in that way it is easier to change or patch the packages.
>
> Creating the two versions would be a lot more work, and in the end
> packages need to be patched anyway.
>
> > Let me know when it's ready, I'll try to upload the new version within
> > the same day that you have the new release ready.
>
> I did it, and briefly tested it with my code.
>
> I think you can upload it and other packages should see no change in
> behaviour for now.

I checked and everything looks all right, but I've been fighting for
1+ hours because it cannot generate the ftgl.pdf documentation.

I'm thinking about dropping it altogether, not sure if anyone will
read this doc in pdf form?  What do you think?

--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
In reply to this post by Evgeny Kapun
Em sex, 8 de fev de 2019 às 01:35, Frank Heckenbach
<[hidden email]> escreveu:

>
> > I checked and everything looks all right, but I've been fighting for
> > 1+ hours because it cannot generate the ftgl.pdf documentation.
>
> Hmm, simply "./autogen.sh && ./configure && make -j" does build it
> for me (though with some warnings), using stretch. Does that work
> for you? If so, it must be something in the Debian rules; otherwise
> it seems to be difference between stable and testing which may be
> worth investigating as it may affect others too. What errors do you
> get?

The compilation doesn't exercise some options if you don't have all
packages installed, like doxygen-gen.

Most of the build is OK, but pdflatex chokes when genereting the .pdf.
You can see the failure here:
  http://debomatic-amd64.debian.net/distribution#unstable/ftgl/2.3.0-3/buildlog

Note that the above is for the previous version, so it must have
happened due to changes in other Debian packages.  The problem might
be in the doc though, that now fails due to more strict tools that are
not going to be changed -- not sure.

In any case, I need to get the package built even if these tools fails
in order to get it into buster.  Dropping the .pdf is the most
straightforward option to achieve it at this point, unless we can find
the real problem in the next hours, and I cannot look at it until the
weekend...

--
Manuel A. Fernandez Montecelo <[hidden email]>

Reply | Threaded
Open this post in threaded view
|

Bug#914153: Update to version 2.3.0-3 breaks Megaglest

Manuel A. Fernandez Montecelo-2
In reply to this post by Manuel A. Fernandez Montecelo-2
Em qua, 21 de nov de 2018 às 01:54, Manuel A. Fernandez Montecelo
<[hidden email]> escreveu:

>
> Hi Evgeny,
>
> 2018-11-19 23:44 Evgeny Kapun:
> >Package: libftgl2
> >Version: 2.3.0-3
> >
> >After updating libftgl2 from version 2.1.3~rc5-5 to version 2.3.0-3, text rendering in Megaglest is broken. Text is shown correctly in menus, but text displayed in the game itself is replaced by white rectangles.
>
> Thanks for the report.
>
> Any idea of what's going on, Frank?
>
> For me it works in some menus, there are white squares in others.  zaz,
> another application using ftgl, seems to work fine, while critterding
> also shows white squares.

Hi again Evgeny,

With the upload of the new upstream 2.4.0-1 I think that this issue
will be fixed and this bug will be closed soon.  Please let us know or
reopen if you still experience similar problems.

Cheers.
--
Manuel A. Fernandez Montecelo <[hidden email]>