[Scummvm-devel] Improvements to blitting code.

Bertrand Augereau bertrand_augereau at yahoo.fr
Mon Aug 18 22:26:38 CEST 2008


Hi Carlo,
I just wanted to check some things about these optimizations :)

First, for the inner loop, it would be interesting to see if the ARM version assumption that there are mostly transparent pixels should be migrated to an early reject before your bit twiddling code (which has a much better average latency than a branchy version assuming random input but is slower in case of a fully transparent scanline).Statistics and measurement should tell.

Considering the acrobatic index fixuping, I don't think this is of any practical use :)
Early returning for (w|h) == 0 then do-whiling and hoisting the fixup constant should be enough without obfuscating the code.

Of course actual performance counter measurements could convince me of the opposite (especially not on a modern x86 that will do intense reordering behind your back) :)

Cheers,
Bertrand


--- En date de : Lun 18.8.08, carlo.bramix <carlo.bramix at libero.it> a écrit :
De: carlo.bramix <carlo.bramix at libero.it>
Objet: Re: [Scummvm-devel] Improvements to blitting code.
À: "scummvm-devel" <scummvm-devel at lists.sourceforge.net>
Date: Lundi 18 Août 2008, 21h13

Hello,
I did a patch, sorry for the delay but I could not do it until now...
This patch does the algorithm that I described previously.
I also did some more additions:
- do...while until the counter reach zero.
- removed the increments to the pointers.
For removing those increments, I used the trick to work in negative logic.
This is an interesting way for optimizing speed critical loops: for example a
loop done in this manner:

for (x=0; x<n; x++)
    p[x] = 0;

can be optimized as:

for (p+=n, n=0-n; n; n++)
    p[n] = 0;

In other words:
1) the pointer 'p' is added with the number of the items to process. If
p=0x1234 and n=4, 'p' is obviously 0x1238.
2) you change the sign of 'n', so it becames negative to -4.
3) now, when you write p[n], you add 'p' and 'n': p=0x1238 and
n=-4, so the resulting address is the first item at address 0x1234.
4) you increment 'n', so it changes from -4 to -3: the expression
'p[n]' now points to the second item because 0x1238-3=0x1235.

I believe this is a good optimization too, because it removes not only the
increments, but also there is no need of the variable 'x' used into the
first "for" cycle.
In the code made for SCUMMVM, it has also the advantage that 'vsPitch'
calculation has been simplified and 'text' too just need to be upgraded
with '_textSurface.pitch' each line.
If you will want to add this method with asmDrawStripToScreen() function too,
you could just use seven parameters instead of eight.

After I downloaded trunk from SVN, I did the fix and then I compiled/tested
SCUMMVM.
Here you are my little report:

============================
I got an error when doing installation:

$ make install
install -d "/home/carlo_pc/inst_scumm/bin"
install -c -s -m 755 "/c/Downloads/trunk/scummvm.exe"
"/home/carlo_pc/inst_scumm/bin/scummvm.exe"
install: cannot stat `/c/Downloads/trunk/scummvm.exe': No such file or
directory
make: *** [install] Error 1

The problem happened because I configured & compiled SCUMMVM from a
different directory.
Since "make install" assumes that scummvm.exe is generated where the
sources are, it cannot find it and it fails.
In my opinion, $EXECUTABLE is always where generated makefile is, so this line
of ports.mk:

$(INSTALL) -c -s -m 755 "$(srcdir)/scummvm$(EXEEXT)"
"$(DESTDIR)$(BINDIR)/scummvm$(EXEEXT)"

should be changed to:

$(INSTALL) -c -s -m 755 "./scummvm$(EXEEXT)"
"$(DESTDIR)$(BINDIR)/scummvm$(EXEEXT)"

Perhaps the most correct value would be $(top_builddir), but I could not find
it...

=====================
It seems to me that asmDrawStripToScreen() function ignores the multiplier, is
it known?
I also found many copies of the some assembly functions, perhaps it could be
better if there will be a /arch-common/arm9 (it's just an example) and into
the platform files you do an "include
arch-common/arm9/DrawStripToScreen.asm".

=====================
My little "feature request":

Into configure script, the selection of an engine is "hardwired", for
example:

add_engine sky "Beneath a Steel Sky" yes

It would be interesting to make it configurable, like:

add_engine sky "Beneath a Steel Sky" $include_sky

and allow the use of an option like --enable-sky for activating the engine
(those variables are assigned to a default value if no option is given on the
command line).
This could be done for all supported engines.
Unfortunately, I'm a (mainly) Win32 developer and I'm still not very
expert with unix stuff, so I doubt I could help you a lot in this task...

=====================

Sincerely,

Carlo Bramini.

---------- Initial Header -----------

>From      : scummvm-devel-bounces at lists.sourceforge.net
To          : max at quendi.de
Cc          : scummvm-devel at lists.sourceforge.net
Date      : Mon, 18 Aug 2008 11:02:50 +0100
Subject : Re: [Scummvm-devel] Improvements to blitting code.

> Hi all,
>
> Interesting technique. I don't claim to entirely understand it yet
> (need to stare at it for a while, I suspect), but it looks very
> hopeful indeed!
>
> In message <2D67F8D4-79FD-4E57-B37F-31457B474337 at quendi.de>
>           Max Horn <max at quendi.de> wrote:
> > P.S.: While I am at it, could somebody (Neil? Robin?) tell me why
> > bot
> >    ./backends/platform/ds/arm9/source/blitters.cpp
> > and
> >    ./engines/scumm/gfxARM.s
> > implement ARM versions of asmDrawStripToScreen and asmCopy8Col? My
> > hope would be that the DS specific one could be removed... ?
>
> I suspect you're right. I think I wrote it originally for the DS and
> it got copied across to the main build.
>
> Until recently I didn't have a DS to build for myself, so someone
> else had to do the building for me.
>
> Blame me for that, anyway, along with the
> USE_ARM_GFX_ASM/USE_ARM_ASM_GFX (or whatever) dyslexia. Sorry.
>
> I'll try and find time to stare at the new ARM code later, but
can't
> promise to do it for this release (I may already be too late). If
> anyone else beats me to it, then so be it.
>
> Thanks,
>
> Robin
> --
> Robin Watts,             Email: <mailto:Robin.Watts at wss.co.uk>
> Warm Silence Software,   WWW:   <http://www.wss.co.uk/>
> P.O.Box 28, Woodstock,   Tel:   01608 737172 (or Mobile: 07885 487642)
> Oxfordshire, OX20 1XX    Fax:   01608 737172
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
challenge
> Build the coolest Linux based applications with Moblin SDK & win great
prizes
> Grand prize is a trip for two to an Open Source event anywhere in the
world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Scummvm-devel mailing list
> Scummvm-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/scummvm-devel






      _____________________________________________________________________________ 
Envoyez avec Yahoo! Mail. Une boite mail plus intelligente http://mail.yahoo.fr
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20080818/63b8e64b/attachment.html>


More information about the Scummvm-devel mailing list