Useless use of float()...

Beitrag von **seife** » Samstag 1. September 2007, 23:26

Hi,

beim Untersuchen von etwas ganz anderem fiel mir der folgende Code in neutrino/src/gui/color.cpp auf:

Index: apps/tuxbox/neutrino/src/gui/color.cpp
===================================================================
RCS file: /cvs/tuxbox/apps/tuxbox/neutrino/src/gui/color.cpp,v
retrieving revision 1.9
diff -u -p -r1.9 color.cpp
--- a/apps/tuxbox/neutrino/src/gui/color.cpp 14 Mar 2004 22:28:13 -0000  1.9
+++ b/apps/tuxbox/neutrino/src/gui/color.cpp  1 Sep 2007 20:40:42 -0000
@@ -38,16 +38,16 @@

 int convertSetupColor2RGB(const unsigned char r, const unsigned char g, const unsigned char b)
 {
-       unsigned char red =     int( float(255./100.)*float(r) );
-       unsigned char green =   int( float(255./100.)*float(g) );
-       unsigned char blue =    int( float(255./100.)*float(b) );
+       unsigned char red =     int(r) * 255 / 100;
+       unsigned char green =   int(g) * 255 / 100;
+       unsigned char blue =    int(b) * 255 / 100;

        return (red << 16) | (green << 8) | blue;
 }

 int convertSetupAlpha2Alpha(unsigned char alpha)
 {
-       return int( float(0x7777/100.)*float(alpha) );
+       return int( 0x7777*int(alpha)/100 );
 }

 void recalcColor(unsigned char &orginal, int fade)
@@ -56,7 +56,7 @@ void recalcColor(unsigned char &orginal,
        {
                return;
        }
-       int color =  int( float( float(orginal) * float( float(fade) / 100.0)) );
+       int color =  int(orginal) * fade / 100;
        if(color>255)
                color=255;
        if(color<0)

Mit meiner Version ist das neutrino-Binary um 328 Bytes kleiner (Bohr ey! :-) und laienhafte Benchmarks zeigen, daß z.B. die Berechnungen in CColorSetupNotifier::changeNotify ca. 30% schneller werden (auf einer dm500s gemessen).[/code]

Beitrag von **seife** » Samstag 1. September 2007, 23:28

Ich habe gleich noch ein paar mehr floats entsorgt.
neutrino-useless-use-of-float.diff
Sollte aber nochmal jemand drüberschauen bzw. ausprobieren, denn ich habs nur auf der dm500 (ohne LCD :-) probiert.

Beitrag von **Houdini** » Sonntag 2. September 2007, 09:30

sieht von aussen erst mal gut aus

Beitrag von **seife** » Sonntag 2. September 2007, 11:12

Bei mir gehts auch, aber wenn es jemand anders auch noch testfährt, kann das sicher nix schaden :-)
Die anderen float()s mache ich auch noch weg, die sind nur nicht soo trivial (und "obviously correct") zu fixen, und außerdem in code, den ich nie benutze (Picviewer), aber wenn wir dann auf "#include <float.h>" verzichten können, dann bringt das sicher einiges an binary-größe.

Hat aber nicht die höchste prio.

Beitrag von **Striper** » Samstag 8. September 2007, 19:09

Läuft 1a auf einer Nokia dbox2!

Beitrag von **seife** » Samstag 8. September 2007, 20:53

Sehr schön, danke für's testen. Hast du auch das Gefühl, daß das etwas "schneller" ist? Insbesondere die Menüs schienen mir etwas "zackiger" aufzugehen, und das infobar-fade-in und fade-out schien "flüssiger" zu sein. Allerdings auf der dreambox, die ja eh schneller ist und außerdem ist das mit dem Gefühl so ne Sache: wenn man eine Stunde an sowas rumgebastelt hat, muß es ja was bringen :-))

Tolle Sache... Wenn das gut läuft ab ins CVS... Tolle Arbeit!!

Beitrag von **Striper** » Samstag 8. September 2007, 22:57

seife hat geschrieben:Sehr schön, danke für's testen. Hast du auch das Gefühl, daß das etwas "schneller" ist?

Also Benches oder so habe ich nicht gemacht. Aber rein theoretisch müsste es eigentlich etwas schneller laufen. Wird schon was übrig bleiben davon in der Praxis

Bin mal gespannt wie es hier weiter geht... Hoffentlich kann mal jemand
Vergleichstest machen...

Gruß
____Paule

SCHIEB NACH OBEN!!

Ich kann nur hoffen das diese Optimierung nicht untergeht...

Gruß
____Paule

Wie spiele ich den Patch denn ein ?
Wollte gestern mal wieder neu bauen. Doch irgentwie
check ich das mit dem diff nÃcht.

Bekomme immer ne Fehlermedung, das die Datei color.cpp
nicht gefunden wird.

Grabber66 hat geschrieben:Wie spiele ich den Patch denn ein ?
Wollte gestern mal wieder neu bauen. Doch irgentwie
check ich das mit dem diff nÃcht.

Bekomme immer ne Fehlermedung, das die Datei color.cpp
nicht gefunden wird.

Versuch's mal mit dem Diff von hier: http://ulc.tuxbox-cvs.sourceforge.net/i ... tory=Diffs

Mit dem aus dem ersten Post (ist eh nur ein Teil) hatte ich den Fehler auch. Ist 'ne kleine Falle, in die man tappt, wenn man nicht alles hier im Thread liest.

Das gelinkte Diff enthält noch mehr Änderungen, also besser das nehmen. (Link ist aus dem zweiten Post in diesem Thread.)

Beitrag von **bellum** » Samstag 22. September 2007, 16:03

seife hat geschrieben:Bei mir gehts auch, aber wenn es jemand anders auch noch testfährt, kann das sicher nix schaden

Habe Dein diff in mein YADD eingebaut und sowohl mit einer Nokia und einer Sagem getestet. Bei beiden sind mir keine Fehler aufgefallen.
Bei der Nokia wars aber nur ein oberflächlicher Kurztest, da Striper ja auch schon getestet hat. Baue mir jetzt für die Sagem ein neues Image und dann geht es in den Lanzeit-Test...
Das mit der Geschwindigkeit ist so ne Sache aber ich würde sagen, dass es sich etwas schneller anfühlt.

seife hat geschrieben:Die anderen float()s mache ich auch noch weg, die sind nur nicht soo trivial (und "obviously correct") zu fixen, und außerdem in code, den ich nie benutze (Picviewer), aber wenn wir dann auf "#include <float.h>" verzichten können, dann bringt das sicher einiges an binary-größe.

Ich benutze den Picviewer, wenn Du die anderen float()s raus gemacht hast, teste ich gerne...

Ach ja nochwas. In epgview.cpp gibt es nach Deinem diff in Zeile 235 noch ein

Code: Alles auswählen

float sbh= (sb- 4)/ sbc;

Ist das so gewollt?

Gruß bellum

Beitrag von **seife** » Samstag 22. September 2007, 16:40

nehmt den:
neutrino-useless-use-of-float-v2.diff

Der sollte die meisten float()s entfernen. Den picviewer habe ich noch nicht ganz "befreit", trotzdem wird das binary kleiner (einige kB).
Ich vermute aber fast, daß es nicht mehr an performancekritischen stellen ist.

Achtung: da ich diesen diff aus meinem "dreambox-experimentierbranch" extrahiert habe, sind da noch ein paar kleinigkeiten drin, die jedoch Hauptsächlich aus "#ifdef HAVE_DREAMBOX_HARDWARE" bestehen bzw. Einfach generische cleanups sind, also nicht stören sollten.

An stellen, wo ich meine Änderungen mittels "#ifdef USE_FLOAT" gekapselt habe, ist das nur deswegen, damit ihr euch anschauen könnt, ob der alte und der neue Code äquivalent sind (sollten sie sein).

DIESER DIFF SOLLTE SO WIE ER IST AUF KEINEN FALL INS CVS.
Aber zum Test sollte er taugen :-)

Beitrag von **bellum** » Mittwoch 3. Oktober 2007, 12:27

seife hat geschrieben:nehmt den:
neutrino-useless-use-of-float-v2.diff

Der sollte die meisten float()s entfernen. Den picviewer habe ich noch nicht ganz "befreit", trotzdem wird das binary kleiner (einige kB).
Ich vermute aber fast, daß es nicht mehr an performancekritischen stellen ist.

Achtung: da ich diesen diff aus meinem "dreambox-experimentierbranch" extrahiert habe, sind da noch ein paar kleinigkeiten drin, die jedoch Hauptsächlich aus "#ifdef HAVE_DREAMBOX_HARDWARE" bestehen bzw. Einfach generische cleanups sind, also nicht stören sollten.

An stellen, wo ich meine Änderungen mittels "#ifdef USE_FLOAT" gekapselt habe, ist das nur deswegen, damit ihr euch anschauen könnt, ob der alte und der neue Code äquivalent sind (sollten sie sein).

DIESER DIFF SOLLTE SO WIE ER IST AUF KEINEN FALL INS CVS.
Aber zum Test sollte er taugen

Hallo Seife,
habe die Änderungen aus Deinem diff händisch (baue noch mit einem CVS Stand vom 13.05.07) bei mir eingebaut und auf meiner Sagen und einer Nokia seit ca. 1 Woche im Test.
Bisher ist mir nichts negatives aufgefallen. Auch der Bildbetrachter funktioniert (auch mit dem PicServer) wie erwartet. Den Audioplayer konnte ich leider nicht testen, da ich diesen nicht benutze. Bzgl. der Geschwindigkeit denke ich, dass die Menüs tatsächlich flotter aufgehen und auch sonst ist die gefühlte Bedienung von Neutrino schneller.
Hat das diff auch noch jemand anders getestet? Können die Änderungen schon ins CVS?

Gruß bellum

Beitrag von **seife** » Mittwoch 3. Oktober 2007, 12:44

Ok, danke für's testen, ich habe es inzwischen auch mal auf meiner Sagem probiert, und da scheint es auch zu gehen. Sobald ich meinen CVS account von derget habe, werde ich das mal Stück für Stück einchecken.

Beitrag von **Houdini** » Mittwoch 3. Oktober 2007, 12:56

meinen CVS account von derget habe

sehr schön, das war ja schon fast lästig

seife hat geschrieben:Sobald ich meinen CVS account von derget habe, werde ich das mal Stück für Stück einchecken.

Ich geb dann eine Runde aus...

Beitrag von **seife** » Mittwoch 3. Oktober 2007, 15:55

OT: weiß eigentlich jemand derget's aktuelle Response-Zeiten? Ich habe am Wochenende angefragt und noch keine Antwort. Nicht daß ich drängeln will, aber bei dem faschistischen Mailfilter der bei meinem "Provider" läuft, kann es schon mal sein, daß da eine Mail nicht angenommen wird, es könnte also sein, daß derget's Antwort bei mir nicht ankam :-)

Beitrag von **Houdini** » Mittwoch 3. Oktober 2007, 16:12

kannst auch mal bei Mws im irc anfragen...

Beitrag von **mb405** » Mittwoch 3. Oktober 2007, 21:25

ich habs auch mal eingebaut. bis auf channellist.cpp hab ich keine negativen auswirkungen feststellen können bis jetzt.
immer wenn ich channellist.cpp irgendwie händisch ändere kann ich keine kanaldirekteingabe mehr machen. da bröselt bei mir immer neutrino ins nirvana ab.
ka woran das liegt.

Beitrag von **seife** » Dienstag 16. Oktober 2007, 13:28

So, ich habe es mal eingecheckt.
@mb405: ich kann dein Problem nicht reproduzieren. Mit "Kanaldirekteingabe" meinst du schon "1", "2" drücken, um auf Kanal 12 zu schalten?

Beitrag von **mb405** » Dienstag 16. Oktober 2007, 17:44

nein bezieht sich nicht auf dein diff, sondern wenn ich mit nen texteditor irgendwas dort drin änder spinnt neutrino und hängt ich bei kanaldirekteingabe auf.
ich nehm kwrite, und überall geht das nur dort nicht. ka warum.

ps ich kann bestätigen, das alles flüssiger läuft damit

Beitrag von **just_me** » Dienstag 23. Oktober 2007, 13:09

Could recalcColor() be further simplified from this one:

Code: Alles auswählen

void recalcColor(unsigned char &orginal, int fade)
{
        if(fade==100)
        {
                return;
        }
        int color =  (int)orginal * fade / 100;
        if(color>255)
                color=255;
        if(color<0)
                color=0;
        orginal = color;
}

to that one:

Code: Alles auswählen

void recalcColor(unsigned char &orginal, unsigned int fade)
{
        original =  orginal * fade / 100;
}

Or is the parameter fade sometimes not within [0..100]?

Greetings,
Frieder

Beitrag von **seife** » Dienstag 23. Oktober 2007, 13:26

Hm i'll look into it. Sounds plausible :-)

Tuxbox-Board

Useless use of float()...

Useless use of float()...

Re: Useless use of float()...