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

 
 

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

Submitter:  Koen Kooi <koenkooi>
Submitted:  Tue 02 Sep 2008 02:47:07 PM UTC
   
 
Category:  render-agg Severity:  3 - Normal
Release:  trunk Status:  Fixed
Privacy:  Public Assigned to:  bjacques
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

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>
Group Member
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>
Group Member
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>
Group 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>
Group Member
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>
Group 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 Files:
   
   
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
  • -email is unavailable- added by bjacques (Posted a comment)
  • -email is unavailable- added by bwy (Posted a comment)
  • -email is unavailable- added by koenkooi (Submitted the item)
  • -email is unavailable- added by koenkooi
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2008-09-04 bjacques StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2008-09-03 bjacques StatusNone Ready For Test
        Assigned toNone bjacques
    2008-09-02 bjacques Attached File- Added trunc.diff, #16421
    2008-09-02 koenkooi Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-caa5.
    Corresponding source code