Some Athens modifications ?

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Some Athens modifications ?

Matthieu
Hello everybody,

I was playing arround with Athens lately and I think I encountered a few small bugs along the way (I may be wrong though) :

1) In LinearGradientPaint class >> from: aStartPoint to: aStopPoint
There is a call to the method "initializeFrom: aStartPoint to: aStopPoint" but this method does not exist. I think the whole method could probably be replaced by :

LinearGradientPaint class >> from: aStartPoint to: aStopPoint
  ^ self new start: aStartPoint; stop: aStopPoint; yourself.

2) This is not much but in the GradientPaint class there is an instance variable named "stops" but it seems that it is never used.

3) In AthensCairoMatrix there is a difference in the variable order between AthensCairoMatrix class >> fieldsDesc and AthensCairoMatrix >> initx: y: sx: sy: shx: shy:

In the first one the order is "sx, shx, shy, sy, x, y" whereas in the second one it is "sx, shy, shx, sy, x, y". However, according to the cairoGraphics API documentation, the order should be the same.

By the way I have a small question : Why not keep the same variable names than in cairoGraphics here (xx, yx, xy, yy, x0, y0) ?

4) In AthensCairoMatrix >> primMultiplyBy: m.

The function as defined in C is the following :
void
cairo_matrix_multiply (cairo_matrix_t *result,
                       const cairo_matrix_t *a,
                       const cairo_matrix_t *b);
It is stated that "The effect of the resulting transformation is to first apply the transformation in a to the coordinates and then apply the transformation in b to the coordinates."

In Pharo it is defined like this :

void   cairo_matrix_multiply (  AthensCairoMatrix * self,
                                           AthensCairoMatrix * m ,
                                           AthensCairoMatrix * self )

For me, when writing "self multiplyBy: aRotationTransformation" it seems more logical to first do the self transformation" and then the rotation transformation (especially if we want a cascade of transformations). However, as currently written in Pharo it is actually the rotation transformation that is applied and then the self transformation.

For example, if I want a scaling then a rotation then a translation it seems easier to me to do :
myTransformation := myTransformation multiplyBy: scalingTransformation.
myTransformation := myTransformation multiplyBy: rotationTransformation.
myTransformation := myTransformation multiplyBy: translationTransformation.
etc. if I want some other transformations.

So it could be nice to have a a method defined like this :

void   cairo_matrix_multiply (  AthensCairoMatrix * self,
                                           AthensCairoMatrix * self ,
                                           AthensCairoMatrix * m )


What do you think about it ?

Thanks,

Matthieu

Reply | Threaded
Open this post in threaded view
|

Re: Some Athens modifications ?

Nicolai Hess


2015-05-06 17:27 GMT+02:00 Matthieu Lacaton <[hidden email]>:
Hello everybody,

I was playing arround with Athens lately and I think I encountered a few small bugs along the way (I may be wrong though) :

1) In LinearGradientPaint class >> from: aStartPoint to: aStopPoint
There is a call to the method "initializeFrom: aStartPoint to: aStopPoint" but this method does not exist. I think the whole method could probably be replaced by : 

LinearGradientPaint class >> from: aStartPoint to: aStopPoint
  ^ self new start: aStartPoint; stop: aStopPoint; yourself.

Yes
 

2) This is not much but in the GradientPaint class there is an instance variable named "stops" but it seems that it is never used.

Yes, stops are part of the color ramp associations.
 

3) In AthensCairoMatrix there is a difference in the variable order between AthensCairoMatrix class >> fieldsDesc and AthensCairoMatrix >> initx: y: sx: sy: shx: shy:

In the first one the order is "sx, shx, shy, sy, x, y" whereas in the second one it is "sx, shy, shx, sy, x, y". However, according to the cairoGraphics API documentation, the order should be the same.

I don't know why i t was done that way.
 

By the way I have a small question : Why not keep the same variable names than in cairoGraphics here (xx, yx, xy, yy, x0, y0) ?

No idea :)
 

4) In AthensCairoMatrix >> primMultiplyBy: m.

The function as defined in C is the following :
void
cairo_matrix_multiply (cairo_matrix_t *result,
                       const cairo_matrix_t *a,
                       const cairo_matrix_t *b);
It is stated that "The effect of the resulting transformation is to first apply the transformation in a to the coordinates and then apply the transformation in b to the coordinates."

In Pharo it is defined like this :

void   cairo_matrix_multiply (  AthensCairoMatrix * self,
                                           AthensCairoMatrix * m ,
                                           AthensCairoMatrix * self )

For me, when writing "self multiplyBy: aRotationTransformation" it seems more logical to first do the self transformation" and then the rotation transformation (especially if we want a cascade of transformations). However, as currently written in Pharo it is actually the rotation transformation that is applied and then the self transformation.

For example, if I want a scaling then a rotation then a translation it seems easier to me to do :
myTransformation := myTransformation multiplyBy: scalingTransformation.
myTransformation := myTransformation multiplyBy: rotationTransformation.
myTransformation := myTransformation multiplyBy: translationTransformation.
etc. if I want some other transformations.

So it could be nice to have a a method defined like this :

void   cairo_matrix_multiply (  AthensCairoMatrix * self,
                                           AthensCairoMatrix * self ,
                                           AthensCairoMatrix * m )


What do you think about it ?

maybe with different names post_multiply / pre_multiply ?

 

Thanks,

Matthieu