A method to obtain the correct-order data from textures
Texture:
(The only caveat is that it would then have to return a const string instead of a PT(uchar))
To continue on this idea, we could even add a format string argument, e.g. the user can specify "RGBA", or even "AAGR" and the code in a function like get_ram_
These don't sound very hard to implement, and I'd be willing to implement them, if it's a good idea.
pro-rsoft
Blueprint information
- Status:
- Complete
- Approver:
- David Rose
- Priority:
- Undefined
- Drafter:
- rdb
- Direction:
- Approved
- Assignee:
- rdb
- Definition:
- Approved
- Series goal:
- Accepted for 1.6.x
- Implementation:
-
Implemented
- Milestone target:
-
1.6.0
- Started by
- rdb
- Completed by
- David Rose
Related branches
Related bugs
Sprints
Whiteboard
Is this the right place to answer? I'm a little bit confused by the launchpad interface. :/
Anyway, sure--it sounds generally useful, and a fine idea. A couple of points come to mind.
1) Even implemented in C++, this might not be fast enough to capture the 3-D results every frame and throw them onto a 2-D window. Of course, CPU's are only getting faster, so maybe performance will be acceptable. No harm in providing the option, as long as people aren't misled into thinking that it's some direct hardware link (which I don't see how they could be).
2) It might be a good idea to return a newly-allocated CPTA_uchar instead of a plain string, just to reduce overhead at the C++ level. The CPTA_uchar can be returned without recopying the string; it will then only get copied once, when the caller calls getData() on the result. However, returning a C++ string object may require copying the entire string twice, once to return from the function, and then again to copy it into the Python string object.
3) Be sure you handle non-standard ram image file formats properly, like alpha, grayscale-alpha, floating-point, even compressed image formats. At the very least, the function should raise an assertion if the actual image format can't be coerced into the requested format, or if there is no ram image available.
David
-------
Yes, I must agree I find it confusing too, but the whiteboard is indeed generally the place to put comments. They really ought to make a commenting interface for blueprints.
I understand that it still might not be suitable for per-frame conversion, but we could clearly state that in the documentation. Hey, it'd be at least faster than in Python. :)
I noticed the compressed ram image stuff too - I assume it isn't just possible to easily read compressed data from the array? In that case, we should throw an error.
Also, what about the name for the function? Something like get_ram_image_as or so, or should we just add a "format" parameter to get_ram_image?
pro-rsoft
-------
When the ram image is compressed, you can certainly read the compressed data easily--the challenge is in uncompressing it. :) While this is possible, I don't see how it would be worth the effort. Throwing an error is sensible, since it doesn't make sense to compress an image you expect to extract to a 2-D API anyway.
I prefer the name get_ram_image_as(). It's clearly different from get_ram_image() which is a simple accessor, and the name implies that the function might do some real work. (I prefer method names which make a distinction between simple accessors and heavyweight data processors.)
David
-------
Okay, I've just committed the get_ram_image_as function. It's pretty flexible, should respect different component widths and weird orders like greyscale-alpha and alpha-only.
Anyhow, thanks a bunch for your help. I'm pretty new to the PTA interface though and am not sure whether I used it right - can you shortly take a peek, and if it looks okay, mark the blueprint as "Implemented"?
Here is a diff: http://
Oh, by the way. I guess I'll also need to duplicate this for get_ram_
pro-rsoft
-------
Looks pretty good at a glance. You might consider adding get_class_type() as the second parameter to PTA_uchar:
The other thing you might consider is duplicating the loop, to handle the common case of _num_components == 1. Virtually every texture in the real world has _num_components == 1, and if you can make that assumption you can avoid the potentially expensive memcpy() call for every pixel, replacing it with a byte assignment instead. I think this optimization is worth the code duplication of the inner loop--it's worth trying it, anyway, to see if it makes a measurable difference on performance. I imagine something like:
if (_num_components == 1) {
for (int p = 0; p < _x_size * _y_size; ++p) {
// do the loop the quick way, with a simple byte assignment.
}
} else {
for (int p = 0; p < _x_size * _y_size; ++p) {
// do the loop the slow way, with memcpy.
}
}
If you really wanted to optimize, it wouldn't be outrageous to write one more special case for the RGBA order, since that is presumably what most callers will want anyway. A special-case loop that knows it is converting 1-component BGRA to RGBA should be able to run substantially faster than the general-purpose loop you have written. In fact, maybe this needs to be the only special case:
if (_num_components == 1 && format == "RGBA") {
for (int p = 0; p < _x_size * _y_size; ++p) {
// a specialized loop to convert 1-component BGRA to RGBA
}
} else {
for (int p = 0; p < _x_size * _y_size; ++p) {
// the original, general loop
}
}
Another minor optimization: I'm not sure you can trust every compiler to know it needs to unroll the _x_size * _y_size operation out of the loop, so it might not be a bad idea to pull it out explicitly, like this:
int num_pixels = _x_size * _y_size;
for (int p = 0; p < num_pixels; ++p) {
Doing so will avoid the need for an extra multiplication operation for each pixel, which can add up.
David
-------
Oh, an additional comment: I forget to answer your question about get_ram_
David
-------
Many thanks for your suggestions. I've just checked in a version that should be more optimized. (The function is pretty long now though.)
(Btw. I assumed you meant to say '_component_width == 1' instead of '_num_components == 1'.)
pro-rsoft
