On Thu, Apr 29, 2010 at 11:45 AM, Bruce Smith <[email protected]> wrote: > On Thu, Apr 29, 2010 at 9:48 AM, Joe Wreschnig <[email protected]> > wrote: >> >> .... >> (I thought we were going to have the discussions of the patches on the >> issue tracker?) >> > > This group is perfect for rapidfire discussions like this one, especially > since they are partly about more general topics than these specific issues. > > Whatever part of this discussion needs to become part of the "permanent > record of the discussion of a specific issue" (according to whoever thinks > it's needed there) can be copied to the issue tracker, or summarized there, > or this thread can be referred to there. > > It's very helpful to have the specific issues broken out as you did, even if > not much of the discussion gets copied into them, as long as the rationale > for the ultimate resolution gets mentioned there. > > == > > As for the specific issues: > > 480 -- I think the "3x speedup" should be mentioned in the issue itself, and > also you should make clear if this is just measured in the specific function > (as your timeit example suggests), or in the speed of some realistic whole > example which uses this function. The issue also claims the current code is > "too slow" without providing any evidence for that. ("Slower than it could > be" does not imply "too slow".)
Maybe you should stick to the areas of your expertise. Any profiling of a game with lots of moving sprites will show invalidate near the top, as the invalidation happens whenever you rebuild the vertex list, which happens when you change any property. > 481 -- sprites are not part of my area of experience in pyglet, but some > general comments: > > - the issue ought to make it completely clear whether or not this is fully > backwards-compatible, > whether a documentation change is required (probably > yes), Which is why I updated the documentation in the patch. > and whether there are any potential downsides that might influence > someone's opinion about this change (e.g. a slowdown, or any change in which > graphics drivers would support the change). Just in case anyone isn't clear, it doesn't slow things down in any measurable way, it is independent of graphics drivers, and it does not break compatibility. It also won't format your hard drive, seduce your spouse, or kill your dog. Instead, it does what it says it does. Enumerating all the problems it *doesn't* have isn't useful. > - the issue doesn't document the new width and height properties and their > relation to the scale properties (if any). Maybe this would be obvious to > someone familiar with pyglet sprites, I don't know. The code describes the properties in detail. > I agree with you that backwards compatibility is very important for this > proposed change. > > 482 -- the issue doesn't document *how* you set position and rotation at the > same time. Since this is a proposed API enhancement, it ought to be > documented in the issue (and ultimately in documentation) and not just in > the new code. The code only adds one new property, and it's documented. > All three issues might benefit from a very clear initial line or phrase > classifying them as "optimization" or "API enhancement" or something else. > (Unless there is an issue tracker field for that which I missed.) Only project maintainers can set issue tracker fields. > For the enhancements which require documentation changes, who will make > those changes? (Or maybe this is already covered, if the function docstrings > are changed and this is sufficient -- I don't know.) I sure hope the documentation is still generated from epydoc, because I added the stupid-looking annotations for it. Maybe this is a difference of expectations. I expect whoever commits my patches to eventually read the code, not the stuff in the issue tracker. The stuff in the issue tracker is there to help searches, let people comment, and link/attach updated patches. It is not a place to repeat everything I did in the code. You seem to expect to never need to even glance at the code of the patches you are vetting. If that's the case, I might as well just be given committer privileges. -- You received this message because you are subscribed to the Google Groups "pyglet-users" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/pyglet-users?hl=en.
