Carsten Neumann wrote: > Hi Marcus, all, > > On Fri, 2007-08-24 at 11:03 +0200, Marcus Lindblom wrote:
>> Just a small comment: Wouldn't it also make sense to make >> imageContentChanged() protected or otherwise hidden from public (but not >> >from Image), so that users don't call it unnecessarily, relying on >> outdated docs/advice/memory. > > Up to this point we have been telling everyone to either call > imageContentChanged or setImage (this one does not really make sense to > me, > beginEditCP(texChunk, TextureChunk::ImageFieldMask); > endEditCP(texChunk, TextureChunk::ImageFieldMask); > should be sufficient/better), so making it protected has the potential > of breaking a lot of code. I'll add some doc comments though. From a user perspective I'd rather not have image-updating knowing about texchunk at all, but I agree we shouldn't break 1.x code. If this patch is applied to 2.0 too, breaking code there might be worth it, no? (See below for an idea to move this function to Image altogether. If we do that, we could remove it from TextureChunk, as it would be redundant.) > Also there is still justification for imageContentChanged, because Image > will cause an update of the whole texture, where a smaller area would > suffice. Hm, now that I've written this, I realize that my patch will > cause performance problems for apps that update only small regions of a > large texture. True. It would perhaps make sense to change this in Image. That would also improve performance for clustering when image-pixel-data is transferred. I don't know if that's a common case though, but if it is, adding "dirty rectangle" logic to Image might be worth it. This might be best put in a new ticket. > ah, no, I got the GL object handling wrong. If only a small region is > updated you can still call imageContentChanged to inform the > TextureChunk about the region that requires a refresh. True. That would work, and it'd be spiffy if you could do this through the parents field. Perhaps it would make sense to put a imageContentChanged() function in Image, that forwards this call to all texturechunk parents? > I'd like to hear opinions whether the patch should still be applied or > if there other ideas on what to do. I think we've got something good here, with very few, if any, drawbacks. > On a related issue, I think that successive calls to imageContentChanged > with non-default arguments should extend the dirty rectangle rather than > replace the previous values. Any comments on this idea? This sounds like a good idea. Do we need separate functions to set and extending the dirty area, with extending being the default, or is that too complex? This means we can also, in the future, enhance this to to update a few small areas separately, should anyone need that. (I'm thinking large virtual textures, etc). Your proposal is compatible with that. mvh /Marcus ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ Opensg-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensg-users
