bugGnash - The GNU Flash player - Bugs: bug #24188, render_handler_agg.cpp still uses...

 
 

bug #24188: render_handler_agg.cpp still uses trunc()

Submitted by:  Koen Kooi <koenkooi>
Submitted on:  Tue 02 Sep 2008 02:47:07 PM UTC  
 
Category: render-aggSeverity: 3 - Normal
Release: trunkStatus: Fixed
Privacy: PublicAssigned to: Bastiaan Jacques <bjacques>
Open/Closed: Closed

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 03 Sep 2008 11:45:52 AM UTC, comment #5:

I've committed (a cleaned up version of) the patch to trunk.

Bastiaan Jacques <bjacques>
Project MemberIn charge of this item.
Tue 02 Sep 2008 06:18:08 PM UTC, comment #4:

Here's a patch that uses boost's numeric conversion to convert to int using truncation. The addition of 0.5 later will convert it back to float. Thoughts?

(file #16421)

Bastiaan Jacques <bjacques>
Project MemberIn charge of this item.
Tue 02 Sep 2008 04:08:26 PM UTC, comment #3:

Hm. Indeed I misread that code.

In that case std::floor would always be one pixel different for negative numbers.

Benjamin Wolsey <bwy>
Project Member
Tue 02 Sep 2008 03:38:03 PM UTC, comment #2:

The point of the truncation is to round the numbers before adding 0.5. This is intended to force the coordinate values to be in between two pixels. I guess std::floor would produce slightly different results, but as long as the results are uniform across the AGG render, I don't see it mattering much.

Bastiaan Jacques <bjacques>
Project MemberIn charge of this item.
Tue 02 Sep 2008 03:33:51 PM UTC, comment #1:

A better idea would be for an agg author to say if or why trunc (and round / rint for that matter) are necessary at all.

If negative numbers are never used (I don't know if this is the case or not), std::floor is always a replacement for trunc. And if it were really important, why always add 0.5 even to negative numbers when truncating? That makes -16.3 into -15...

I would suggest replacing all non-standard rounding functions with std::floor(x + 0.5), unless there's a good reason not to.

We don't want to use non-standard functions like trunc.

Benjamin Wolsey <bwy>
Project Member
Tue 02 Sep 2008 02:47:07 PM UTC, original submission:

8blender_rgb555_preENS1_12row_accessorIhEEEEE14draw_poly_implINS1_11scanline_p8EEEvPKNS_8geometry7Point2dIfEEmRKNS_4rgbaESH_RT_RKNS_6matrixE[void gnash::render_handler_agg<agg::pixfmt_alpha_blend_rgb_packed<agg::blender_rgb555_pre, agg::row_accessor<unsigned char> > >::draw_poly_impl<agg::scanline_p8>(gnash::geometry::Point2d<float> const*, unsigned long, gnash::rgba const&, gnash::rgba const&, agg::scanline_p8&, gnash::matrix const&)]+0x1b2): more undefined references to `trunc' follow

./backend/render_handler_agg.cpp:1823: point(trunc(corners[0].x), trunc(corners[0].y)));
./backend/render_handler_agg.cpp:1824: path.move_to(trunc(origin.x)+0.5, trunc(origin.y)+0.5);
./backend/render_handler_agg.cpp:1830: path.line_to(trunc(pnt.x)+0.5, trunc(pnt.y)+0.5);
./backend/render_handler_agg.cpp:1834: path.line_to(trunc(origin.x)+0.5, trunc(origin.y)+0.5);

koen@dominion:/OE/monotone/org.openembedded.dev/packages/gnash$ strings /OE/angstrom-tmp/staging/arm-angstrom-linux-gnueabi/usr/lib/libm.so | grep trunc
trunc
truncl
truncf
koen@dominion:/OE/monotone/org.openembedded.dev/packages/gnash$ strings /OE/angstrom-tmp/staging/avr32-angstrom-linux-uclibc/usr/lib/libm.so.0 | grep trunc
koen@dominion:/OE/monotone/org.openembedded.dev/packages/gnash$

avr32 has no 'trunc' function in libm, while arm does have it.

The commits that broke it are:

revision 1.538
date: 2008-05-27 09:51:59 +0200; author: bwy; state: Exp; lines: +5 -5; commitid: knu3JdhLjZlqLz4t;

* configure.ac: drop tests for unused C library functions.

2008-05-17 Benjamin Wolsey <bwy@benjaminwolsey.de>

* libbase/utility.h: drop LN_2, log2, trunc replacements as they
aren't used anywhere in the code. Move exp2 define to
MovieTester.cpp as it's only used there.
* testsuite/MovieTester.{cpp,h}: add inline exp2 function in case
it's missing.

The fix is quite simple:

remove the 'dnl' from the AC_CHECK_LIB(m, trunc... in configure.ac

And readd this to libbase/utility.h:

#ifndef HAVE_TRUNC
inline double trunc(double x) { return (x < 0 ? -(std::floor(-x)) : std::floor(x)); }
#endif

Koen Kooi <koenkooi>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #16421:  trunc.diff added by bjacques (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by bjacques (Posted a comment)
  • -unavailable- added by bwy (Posted a comment)
  • -unavailable- added by koenkooi (Submitted the item)
  • -unavailable- added by koenkooi
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 04 Sep 2008 06:34:14 AM UTCbjacquesStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Wed 03 Sep 2008 11:46:08 AM UTCbjacquesStatusNone=>Ready For Test
      Assigned toNone=>bjacques
    Tue 02 Sep 2008 06:18:07 PM UTCbjacquesAttached File-=>Added trunc.diff, #16421
    Tue 02 Sep 2008 02:47:07 PM UTCkoenkooiCarbon-Copy-=>Added -unavailable-

    Back to the top


    Powered by Savane 3.1-cleanup1