aKademy 2006 BoF session ideas

> What happens if we eat a coin but don't update the
status bar to
> keep score (disable signal)?  Do we still get the full
re-draw?

Yes, the statusbar was my main suspect, but it is not the
source of the
issue. If you want to see it, build SVN (there is one other
optimization I
commited today), make && make install, open a
Konsole windows and

export QT_FLUSH_PAINT=1
kgoldrunner

Every time you eat a coin or dig you will get a full
repaint. You can try
disabling the statusbar, but it will be there yet, so do not
bother with
it. I have traced it to the setPixmap() call in setTile. It
occurs on both
QGV and the light canvas class. I will try to find the
source of it
tomorrow after the BoF, but if you (or Maurizio) have some
free time
please try the test outlined above. You can see it even in
level 1. Just
stay in place, and click the mouse to dig (left or right).
You will get
one full repaint for each dig stage. I confirmed that only
one setTile is
being called.

Maurizio, if you have time, could you use the technique
above and check if
Pixmap->setPixmap() is not triggering a full redraw of
the canvas, and not
only the lastrect and the new one, just to be sure? I will
investigate it
myself as well later when I am more rested.

Regards,
Mauricio

_______________________________________________
kde-games-devel mailing list
kde-games-develkde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel

> Maurizio, if you have time, could you use the technique
above and check if
> Pixmap->setPixmap() is not triggering a full redraw
of the canvas, and not
> only the lastrect and the new one, just to be sure? I
will investigate it
> myself as well later when I am more rested.

OK! Found it, finally! The problem is that if your tile is
in a group,
then changed() will invoke Group::changedInternal() (In
KGoldrunner this
is KGrGameCanvasGroup). Look at the implementation and this
will mark as
members of the groups as changed, which in the case of
KGoldRunner means
that marking one tile as changed will dirty the whole field.

A quick fix is in in KGrGameCanvas.h, if you want to use it
in KBoard to
test just change the inline implementation of changed to
something like

void changed() { KGrGameCanvasItem::changedInternal(); }

Or maybe we should instead change in all item classes

if(visible() && canvas() )
    changed();

to

if(visible() && canvas() )
    KGrGameCanvasItem::changed();

Let me know what you think is the best solution, Maurizio. I
am a bit
tired right now, need to take a nap!

Regards,
Mauricio Piacentini

BTW, I was seeing the same in QGV, so maybe it uses a
similar design, and
has the same issue? Will investigate it.





_______________________________________________
kde-games-devel mailing list
kde-games-develkde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel

Correction to the previous email (typo)

> is KGrGameCanvasGroup). Look at the implementation and
this will mark as
> members of the groups as changed, which in the case of
KGoldRunner means

Should be

> is KGrGameCanvasGroup). Look at the implementation and
this will mark ALL
> members of the groups as changed, which in the case of
KGoldRunner means

And of course this is just a workaround for testing, we
should probably
remove the inline in order to avoid the inheritance problem,
or call the
right version directly from the items.

Regards,
Mauricio Piacentini

_______________________________________________
kde-games-devel mailing list
kde-games-develkde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel

Hi, unluckily i never received the mail you are answering,
where was it sent?
For the bug, i'm quite sure that Pixmap::setPixmap will not
update the
entire group,
does your patch really improves performance?
And BTW, that patch breaks the case of a group being moved,
because in this
case all the items have to be updated.
I you want to debug the canvas updates, i recommend you to
set the
DEBUG_CANVAS_PAINTS define in canvas.cpp to 1, so you will
have some
visual feedback (if you are somewhat masochist put also
DEBUG_DONT_MERGE_UPDATES to 1, this way you will see
independently all
the operations that modify the canvas)
I also have a very small update in the kboard svn (in
playground/kboard/src/canvas)
that unifies the changedInternal  and changed functions
(just a cleanup, they do
the same thing anyway).
Have fun! 

On 9/26/06, mauriciotabuleiro.com <mauriciotabuleiro.com> wrote:
> > Maurizio, if you have time, could you use the
technique above and check if
> > Pixmap->setPixmap() is not triggering a full
redraw of the canvas, and not
> > only the lastrect and the new one, just to be
sure? I will investigate it
> > myself as well later when I am more rested.
>
> OK! Found it, finally! The problem is that if your tile
is in a group,
> then changed() will invoke Group::changedInternal() (In
KGoldrunner this
> is KGrGameCanvasGroup). Look at the implementation and
this will mark as
> members of the groups as changed, which in the case of
KGoldRunner means
> that marking one tile as changed will dirty the whole
field.
>
> A quick fix is in in KGrGameCanvas.h, if you want to
use it in KBoard to
> test just change the inline implementation of changed
to something like
>
> void changed() { KGrGameCanvasItem::changedInternal();
}
>
> Or maybe we should instead change in all item classes
>
> if(visible() && canvas() )
>     changed();
>
> to
>
> if(visible() && canvas() )
>     KGrGameCanvasItem::changed();
>
> Let me know what you think is the best solution,
Maurizio. I am a bit
> tired right now, need to take a nap!
>
> Regards,
> Mauricio Piacentini
>
> BTW, I was seeing the same in QGV, so maybe it uses a
similar design, and
> has the same issue? Will investigate it.
>
>
>
>
>
> _______________________________________________
> kde-games-devel mailing list
> kde-games-develkde.org
> https://mail.kde.org/mailman/listinfo/kde-games-devel
>


-- 
Ciao
Maurizio

"Well we all shine on
Like the moon and the stars and the sun" (John Lennon)
_______________________________________________
kde-games-devel mailing list
kde-games-develkde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel

Maurizio Monge wrote:
> Hi, unluckily i never received the mail you are
answering, where was it
sent?

I believe all messages in this thread can be found at
kde-games-devel
archives : http://lists.
kde.org/?l=kde-games-devel

The discussion started on the previous thread
Re: [Kde-games-devel] aKademy 2006 BoF session ideas

> For the bug, i'm quite sure that Pixmap::setPixmap will
not update the
> entire group,

Well, it should not, but this is not what I am seeing when I
run
KGoldrunner. I placed trace information in several places
and confirmed
it. Just revert the last one line patch in KGoldRunner SVN,
and run the
previous version with

export QT_FLUSH_PAINT=1

and you will see that it does mark all members of the
background group as
m_changed when you setPixmap on only one. Did you try it?

> does your patch really improves performance?

Yes, just a bit, in this specific situation only. It was
already good, but
avoiding the update of the 700 tiles that have not moved
avoids spikes on
lower end machines or ones with older video subsystems, and
makes it
optimal.

> And BTW, that patch breaks the case of a group being
moved, because in this
> case all the items have to be updated.

Yes, I know. I mentioned this in the last email, probably a
redesign on
the inline changed() implementation was more appropriate
than my one line
fix. Or each class (Pixmap, TiledPixmap) should call
KGrGameCanvasItem::changed() directly. But the one line fix
shows you the
difference in updates, with it only one item (the one that
had the
setPixmap change) changedInternal() is called. Otherwise the
group
changeInternal() is called, and it marks all items are
changed.

> I you want to debug the canvas updates, i recommend you
to set the
> DEBUG_CANVAS_PAINTS define in canvas.cpp to 1, so you
will have some
> visual feedback (if you are somewhat masochist put also
> DEBUG_DONT_MERGE_UPDATES to 1, this way you will see
independently all
> the operations that modify the canvas)

DEBUG_DONT_MERGE_UPDATES caused a crash in KGoldrunner when
set (before
the patch), with a segmentation fault. It works if you queue
the metacall
only, but this is similar to calling the slot:

#if DEBUG_DONT_MERGE_UPDATES
  //updateChanges(); << segmentation fault here
  //workd if you change it to
  QMetaObject::invokeMethod(this, "updateChanges",
Qt::QueuedConnection);
#else //DEBUG_DONT_MERGE_UPDATES
  QTimer::singleShot( 0, this, SLOT(updateChanges()) );
#endif //DEBUG_DONT_MERGE_UPDATES

But of course, you will not see the individual updates any
more 
But you
will see the app.

Anyway, you can see updates (global) by exporting the
environment variable
I mentioned above, even in release builds. Very useful, try
it!

> I also have a very small update in the kboard svn (in
> playground/kboard/src/canvas)
> that unifies the changedInternal  and changed functions
(just a cleanup,
they do
> the same thing anyway).

Well, maybe this solves the inheritance issue that was
causing the problem
in the first place. I will see if I can get it from SVN to
compare, or I
will simply repatch the kgoldrunner version. Maybe this is
why you were
not seeing this in KBoard, or maybe the usage pattern is a
bit different.

Regards,
Mauricio Piacentini


_______________________________________________
kde-games-devel mailing list
kde-games-develkde.org
https://mail.kde.org/mailman/listinfo/kde-games-devel