pros: - spare some calls to sin / cos - multiplied by the times _update_position is called in a frame
cons: - it adds state to manage. By example, the update_position (without underscore) should also be changed to update the cached values - what it was a trivial setter now needs to be read to see if there is some surprise - while reading _update_position code, the reader will need to seek what _rotation_cr and _rotation_sr are, and how they work I would benchmark some cases, if perf increase turns to be too low it may not be worth. I will try to get some numbers to talk. On Mon, Apr 10, 2017 at 10:18 PM, Benjamin Moran <[email protected]> wrote: > Great, thanks for your feedback. Also, thanks for starting this thread. > I'm going to merge in these changes. > > Also, while your here, maybe I can ask your opinion about some additional > changes. > Currently, the Sprite._update_position method might perform some wasted > math when dealing with rotated sprites. If a sprite is rotated, the > `radian`, `sin` and `cos` calculations are done whenever the sprite is > moved - even if you don't change the rotation. In this case, we can cache > the "cr", "sr" variables. > > If we cache these values, it will be faster in cases where the sprite is > only rotated one time. I'm thinking to change it to this: > > elif self._rotation: > .......... > cr = self._rotation_cr > sr = self._rotation_sr > > > And move the logic into the @rotation.setter: > > @rotation.setter > def rotation(self, rotation): > self._rotation = rotation > r = -math.radians(rotation) > self._rotation_cr = math.cos(r) > self._rotation_sr = math.sin(r) > self._update_position() > > > > > On Tuesday, April 11, 2017 at 12:39:57 AM UTC+9, claudio canepa wrote: >> >> Yes, your code applies the cocos style. >> Sorry for the noise, I skipped the scale_z = _scale * _scale_z, so the >> calcs below looked as it was missing on _scale. I blame the late night, >> but anyway sorry. >> >> Looks good to me. >> >> >> >> >> On Mon, Apr 10, 2017 at 3:41 AM, Benjamin Moran <[email protected]> >> wrote: >> >>> Hi Claudio, >>> >>> The changes I have made function exactly as you describe (like the >>> CocosNode description). >>> The direct link to the changed sprite.py is here (in my personal fork): >>> https://bitbucket.org/HigashiNoKaze/pyglet/src/738617edac87a >>> 5e313414b790db412763983524e/pyglet/sprite.py?fileviewer= >>> file-view-default >>> >>> >>> On Monday, April 10, 2017 at 3:13:43 PM UTC+9, claudio canepa wrote: >>> >>>> >>>> >>>> On Sun, Apr 9, 2017 at 11:12 PM, Benjamin Moran <[email protected]> >>>> wrote: >>>> >>>>> Hi Claudio, >>>>> >>>>> [snip] >>>> >>>>> >>>>> The new behavior has not be included in any release yet, and is not >>>>> yet documented. I think it's fine to consider changes, as long as we don't >>>>> break the usage of `scale`. I like your suggestion. To summerize, the >>>>> behavior would be: >>>>> >>>>> - The `scale` attribute will become a "base scale" value. >>>>> - The `scale_x` and `scale_y` attributes will interpolate from >>>>> `scale`, if it is != 1. >>>>> >>>>> I think this matches the cocos behavior, correct? >>>>> >>>>> Mmm. "Interpolate" does not sound as the cocos behavior; cocos simply >>>> scales x axis by _scale * _scale_x and y axis by _scale * _scale_y. >>>> >>>> The CocosNode description for the relevant members says >>>> >>>> #: a float, alters the scale of this node and its children. >>>> #: Default: 1.0 >>>> self._scale = 1.0 >>>> >>>> #: a float, alters the horizontal scale of this node and its >>>> children. >>>> #: total scale along x axis is _scale_x * _scale >>>> #: Default: 1.0 >>>> self._scale_x = 1.0 >>>> >>>> #: a float, alters the vertical scale of this node and its >>>> children. >>>> #: total scale along y axis is _scale_y * _scale >>>> #: Default: 1.0 >>>> self._scale_y = 1.0 >>>> >>>> """ >>>> >>>> I would say that image.width * scale_x and image.height * scale_y gives >>>> the "resized base image" we want to use, which is furter resized by _scale >>>> >>>> - scale usage is backward compatible with latest pyglet released >>>> >>>> - Sprites that do not need to change the width / height ratio provided >>>> in the image don't need to fiddle with scale_x or scale_y, resize only >>>> needs to change the scalar '_scale' >>>> >>>> - A Sprite that wants to adjust the width / height ratio at creation >>>> time sets the appropiate scale_x and scale_y; if the ratio does not changes >>>> after creation then it does not need to further touch scale_x nor scale_y. >>>> Changing _scale along time will resize the initial displayed image, >>>> preserving the ratio defined at creation. >>>> >>>> - for special effects when dinamic changes in the ratio are desired, >>>> scale_x and scale_y are available. >>>> >>>> To see it in code, the relevant part would be _update_position in >>>> cocos.sprite.Sprite, at https://github.com/los-cocos/c >>>> ocos/blob/master/cocos/sprite.py#L326 >>>> >>>> BTW, your next mail says you pushed a variation based in your >>>> interpretation to your repo; I could't find there. Maybe it wasn't pushed ? >>>> >>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "pyglet-users" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to [email protected]. >>> To post to this group, send email to [email protected]. >>> Visit this group at https://groups.google.com/group/pyglet-users. >>> For more options, visit https://groups.google.com/d/optout. >>> >> >> -- > You received this message because you are subscribed to the Google Groups > "pyglet-users" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at https://groups.google.com/group/pyglet-users. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "pyglet-users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/pyglet-users. For more options, visit https://groups.google.com/d/optout.
