From: Kouhei S. <ko...@co...> - 2009-08-02 09:14:49
|
Hi, In <a5d...@ma...> "Re: [ruby-gnome2-devel-en] cannot set pixbuf pixels - how to modify an image?" on Sun, 2 Aug 2009 08:37:15 +0200, Michal Suchanek <hra...@ce...> wrote: >>> Which I resolved by patching the pixbuf bindings. >> >> set_pixels: >> >> * We should use RSTRING_PTR() instead of RSTRING()->ptr >> and RSTRING_LEN() instead of RSTRING()->len. >> * We should return self for setter method: >> http://ruby-gnome2.sourceforge.jp/hiki.cgi?Naming+and+Conversion+Rules#Accessors+%28Setter%2FGetter+methods%29 > > OK, these are easy to change, I just used whatever worked (and > whatever was described in README.EXT) Please. >> * Why memmove()? Should we use memcpy()? > > memmove is the safer variant. If the user got to the original pointer > somehow the copied regions might overlap. While it's unlikely one > more check should not impact performance and should avoid some > interesting bugs. It seems that the user can't get the original pointer. If the user can get the original pointer, the user doesn't need to set pixels. The user can only change the original pointer. So I think that we can use memcpy(). >> * Should we require "pixbuf.pixels.size == new_pixels.size"? > > I guess it's OK to set only part of the pixels. I am not sure > returning error is appropriate if the sizes do not match. If we provide an 'offset' argument, it will be reasonable. But set_pixels doesn't provide it. So I think that the sizes should be matched. >> get_pixel, set_pixel: >> >> * Are they needed? > > When you do not want to set all pixels of a large bitmap they might be > faster. And they give the high-level interface so they should be the > primary access method for high-level language like ruby. > > They should be probably converted to match the color format used by > other methods,though. When they work as you said, we should consider about {get,set}_pixel. Is it OK? Anyway we should treat get_pixels and {get,set}_pixel with separated patches. Thanks, -- kou |