Half-usable diff from downstream

Hi,

while working on ensuring that kalzium keeps running well on software-only
OpenGL, I had to make some changes to our snapshot of libavogadro: see
attached diff.

I’m not committing to avogadro because:

  • you probably don’t want all of that, e.g. the CMakeLists change;
  • you might not want to disable the zoom eyecandy as I did (I had to because I
    don’t want us to be swarmed by user complaints about that, but in avogadro
    itslef it’s less of an emergency and you have time to improve it).
  • I added a member to GLWidget, namely a quality setting (whereas until now
    the quality setting was only stored in the painter). It’s not optimal as the
    quality setting is now stored at two places! I added it to GLWidget because
    it is now used to determine whether or not to enable GL_LIGHT1. So it made
    more sense to have it here. So, I don’t know what’s the optimal solution.
    Maybe keep it in GLWidget and remove it in Painter. I don’t know!

So I’m just attaching this diff FYI.

Cheers,

Benoit

I forgot to mention: one more thing that diff does is to disable all eyecandy
when quality<=1.

Cheers,

Benoit

On Tuesday 08 April 2008 12:09:31 Benoît Jacob wrote:

Hi,

while working on ensuring that kalzium keeps running well on software-only
OpenGL, I had to make some changes to our snapshot of libavogadro: see
attached diff.

I’m not committing to avogadro because:

  • you probably don’t want all of that, e.g. the CMakeLists change;
  • you might not want to disable the zoom eyecandy as I did (I had to
    because I don’t want us to be swarmed by user complaints about that, but in
    avogadro itslef it’s less of an emergency and you have time to improve it).
  • I added a member to GLWidget, namely a quality setting (whereas until now
    the quality setting was only stored in the painter). It’s not optimal as
    the quality setting is now stored at two places! I added it to GLWidget
    because it is now used to determine whether or not to enable GL_LIGHT1. So
    it made more sense to have it here. So, I don’t know what’s the optimal
    solution. Maybe keep it in GLWidget and remove it in Painter. I don’t know!

So I’m just attaching this diff FYI.

Cheers,

Benoit

On Tuesday 08 April 2008 07:24:53 Benoît Jacob wrote:

I forgot to mention: one more thing that diff does is to disable all
eyecandy when quality<=1.

On Tuesday 08 April 2008 12:09:31 Benoît Jacob wrote:

while working on ensuring that kalzium keeps running well on
software-only OpenGL, I had to make some changes to our snapshot of
libavogadro: see attached diff.

I’m not committing to avogadro because:

  • you probably don’t want all of that, e.g. the CMakeLists change;
  • you might not want to disable the zoom eyecandy as I did (I had to
    because I don’t want us to be swarmed by user complaints about that, but
    in avogadro itslef it’s less of an emergency and you have time to improve
    it). - I added a member to GLWidget, namely a quality setting (whereas
    until now the quality setting was only stored in the painter). It’s not
    optimal as the quality setting is now stored at two places! I added it to
    GLWidget because it is now used to determine whether or not to enable
    GL_LIGHT1. So it made more sense to have it here. So, I don’t know what’s
    the optimal solution. Maybe keep it in GLWidget and remove it in Painter.
    I don’t know!

The GLWidget has a pointer to the painter so why not just call the painter
quality method so that you can check what the painter’s quality level is and
act accordingly? What I have been trying to do with the Kalzium port is
resist the urge to add anything to the API unless absolutely necessary so
that when it starts to use the system copy no/little work will need to be
done.

I did have it on my list to do similar though and I will see about porting
your changes across. I was thinking only one light source in low quality and
no eye candy. With the cursor change stuff I added recently it isn’t quite
as bad when the eye candy is turned off now.

On Tuesday 08 April 2008 13:55:24 Marcus D. Hanwell wrote:

The GLWidget has a pointer to the painter so why not just call the painter
quality method so that you can check what the painter’s quality level is
and act accordingly? What I have been trying to do with the Kalzium port is
resist the urge to add anything to the API unless absolutely necessary so
that when it starts to use the system copy no/little work will need to be
done.

You’re perfectly right. I should have resisted too!

I did have it on my list to do similar though and I will see about porting
your changes across. I was thinking only one light source in low quality
and no eye candy. With the cursor change stuff I added recently it isn’t
quite as bad when the eye candy is turned off now.

Sorry then, I shouldn’t have interfered here.

I’ll revert my changes and come up with a better solution.

Benoit

On Tuesday 08 April 2008 08:08:33 Benoît Jacob wrote:

On Tuesday 08 April 2008 13:55:24 Marcus D. Hanwell wrote:

The GLWidget has a pointer to the painter so why not just call the
painter quality method so that you can check what the painter’s quality
level is and act accordingly? What I have been trying to do with the
Kalzium port is resist the urge to add anything to the API unless
absolutely necessary so that when it starts to use the system copy
no/little work will need to be done.

You’re perfectly right. I should have resisted too!

I didn’t mean to criticise - more to point out why I have done things in
certain ways.

I did have it on my list to do similar though and I will see about
porting your changes across. I was thinking only one light source in low
quality and no eye candy. With the cursor change stuff I added recently
it isn’t quite as bad when the eye candy is turned off now.

Sorry then, I shouldn’t have interfered here.

I’ll revert my changes and come up with a better solution.

Again the changes look good - I was just planning on tweaking them when I got
a chance later. I am usually pleased when someone beats me something on my to
do list as there is always more I want to do :wink:

Hi,

I committed the a better solution to kalzium, attached is the diff (from
r794593 i.e. Marcus’s latest commit if I’m not mistaken).

Now I only use (and don’t modify) quality() and setQuality() from GLWidget.

There was a big problem that I would like to mention:

setQuality() only changes d->newQuality in the GLPainter, while quality()
reads d->quality.

Both values are synced when GLPainterPrivate::isValid() is called, which is
whenever an object is drawn.

But in order to turn on/off LIGHT1, I needed to read the quality before
drawing any object.

So I did this little workaround: I let GLPainter::begin() call d->isValid().
And then in GLWidget::render() I only read quality() after having called
d->painter->begin(). I hope this doesn’t affect performance significantly.

That doesn’t feel very solid… but I don’t see what else I could do without
changing the API.

Perhaps a better solution would be to offer a GLWidget constructor taking a
quality argument.

Cheers,

Benoit