Re: [Gnash-commit] gnash ChangeLog libbase/utility.h server/charac...

[zou lunkai]

On 6/14/08, Benjamin Wolsey <address@hidden> wrote:
> > >
> >
> > what about 'double' to 'float' conversion? Is it also undefined?
> >
>
> Float-to-float conversion:
>
> An rvalue of floating point type can be converted to an rvalue of
> another floating point type. If the source value can be exactly
> represented in the destination type, the result of the conversion is
> that exact representation. If the source value is between two adjacent
> destination values, the result of the conversion is an
> implementation-defined choice of either of those values. Otherwise, the
> behavior is undefined.
>
> So yes, if the double is outside the range of the float type.
>
> > FloatToFixed16() and DoubleToFixed16() are only used for matrix at the
> > moment, and they are mainly for converting cos() and sin() values to
> > integers, so shouldn't be a problem(all in valid ranges). I may move
> > them inside the matrix class later.
> >
> That would surely be a part of a good answer, so that no one is tempted
> to use them generally. Perhaps documenting the range of their allowed
> usage would also be useful.
>
> I see that the cos/sin values are multiplied by scale first before being
> passed to FloatToFixed16. If the maximum allowable value for
> FloatToFixed16 is 2**32 / 65535.0, it means the maximum scale is about
> 65537. It sounds like this would be an unusual value for scale - is that
> correct?
>

The maximum scale for AS(eg. movieclip._xscale) could be much bigger
than 65537(tested). The scale item in matrix should follow into 16.16
format integer(deduction based on tests). The two items are related
but not the same thing.  That's the next task for the core. And I
think the solution is out of the matrix itself.

eg. see this function:

float
matrix::get_x_scale() const
{
    return sqrtf(((float)sx * sx + (float)shx * shx)) / 65536.0f;
}

get_x_scale() always returns a positive scale value no matter what
data types are used for the matrix itself, which is dominated by
mathematics equation.  But for AS, the scale value could be a negative
one. see tests in matrix_accuracy_test1.sc

        mc1._xscale = -1;   // triggers matrix::set_x_scale(float
xscale) directly
        xcheck_equals(mc1._xscale, -1); // triggers get_x_scale() directly.

See?  the solution need a appropriate AS engine at high level, instead
of inside the matrix itself.

Comments, strk?


> > PIXEL_TO_TWIPS() then need to be inspected if it is really unsafe.
>
> It also seems very unlikely that this will be a common problem. The
> lowest pixel value that would cause a problem would be 107 million.
>
> In both cases, how much of a performance hit would it be to test the
> number for an overflow and reject it? Would that give correct behaviour?
>

If the number is overflow, you should truncate it instead of reject.
eg. the code might looks like this(not accurate):

inline int PIXELS_TO_TWIPS(double d)
{
      if(d > 0x7fffffff || d < -0x7fffffff)
       {
              return  20 * fmod(d,  0x7fffffff);
       }
       else
       {
             return 20 * d;
       }
}



All I know is that the above function is much slower, but cann't
evaluate how much performance lost would be for the overall
player(tests dependent).  Just like that I cann't evaluate how much
overall performance imporvement the new integer matrix would bring for
us. But I am sure the matrix itself is much faster.

The overall performance up should be reached through a step by step
way, it's hard to demonstrate how big performance achievement or
regression is made if only a small part changes in Gnash. I really
don't know which single part makes Gnash slow, especially for the
non-as core:) I rely on inaccurate theoretical analysis. Drop float
matrix, float cxform, float edges, float paths, float rect, float
invalidated bound detecting...  all seems good and correct direction
to me.

> The final question that occurs to me is whether a negative float is
> 'representable' in an unsigned value, or whether any negative value for
> the float is undefined behaviour when casting to unsigned int.
>

It's undefined according to your docs.  But for matrix only cases, we
don't need the 'unsigned' cast now.

For fun, any knows for which compiler and CPU, the result of the
following code is not  4294967295(0xffffffff)???

uint32 d = (uint32)(-1.0);
std::cout << d << std::endl;

--zou