Item render-to-texture: Texture modifier, speedup #10570
Conversation
| @@ -798,6 +798,10 @@ autoscale_mode (Autoscaling mode) enum disable disable,enable,force | |||
| # A restart is required after changing this. | |||
| show_entity_selectionbox (Show entity selection boxes) bool false | |||
|
|
|||
| # Speed up item rendering through RTT (render-to-texture) | |||
| # Set to 0 to disable, numbers > 0 are resolutions in pixels | |||
| item_rtt_speedup_resolution (Item render-to-texture speedup) int 0 0 1024 | |||
There was a problem hiding this comment.
Default is 128 (not 0), right?
| if (w != "") { | ||
| width = stoi(w); | ||
| const std::string h = sf.next(""); | ||
| if (h != "") |
There was a problem hiding this comment.
| if (h != "") | |
| if (!h.empty()) |
| const std::string w = sf.next("x"); | ||
| u32 width = g_settings->getU16("item_rtt_speedup_resolution"); | ||
| u32 height = width; | ||
| if (w != "") { |
There was a problem hiding this comment.
| if (w != "") { | |
| if (!w.empty()) { |
|
|
||
| rt->unlock(); // wondering whether this is required | ||
| driver->removeTexture(rt); | ||
| // rt->drop(); isn't required |
There was a problem hiding this comment.
If it must not be dropped, it's a good notice to programmers. However, wondering whether this is required is much worse for being unclear. Perhaps this needs a valgrind run to ensure there are no memory leaks.
| width = stoi(w); | ||
| const std::string h = sf.next(""); | ||
| if (h != "") | ||
| height = stoi(h); |
There was a problem hiding this comment.
height and width aren't zero and print a message to error stream if they are?
|
What types of video cards don't have support for render textures? Irrlicht's documentation implies that this does not have universal support. I don't know how common it is to not have support, but if it's more than just a very few, it should be documented that it may not work depending on the video card. |
|
|
||
| * If `[item:<modname>:<itemname>` is used, width & height default to `item_rtt_speedup_resolution` (client setting) | ||
| * If `[item:<modname>:<itemname>:W` is used, width & height are set to W | ||
| * If `[item:<modname>:<itemname>:WxH` is used, width is set to W & height is set to H |
There was a problem hiding this comment.
Item names may be in this format, but it's not enforced. [item:air is a valid item. Thus consider using , as second argument delimiter, despite being inconsistent to other modifiers.
There was a problem hiding this comment.
Yeah, I'm currently considering using [item:<serialized_itemstack>:WxH
|
|
||
| core::matrix4 ViewMatrix; | ||
| ViewMatrix.buildProjectionMatrixOrthoLH(2.0f, 2.0f, -1.0f, 100.0f); | ||
| ViewMatrix.setTranslation(core::vector3df( |
There was a problem hiding this comment.
Why copy & paste? tile.cpp and hud.cpp are both client files, thus it would make sense to move this code to a separate function which both files can access.
There was a problem hiding this comment.
@pyrollo noted the same.
code redundancy still needs to be fixed
|
Please rebase and address the remaining comments. I'd like to see it merged as it speedup the inventory rendering by a lot (versus the drawItemStack scene per item). |
@kilbith Currently I'm busy. Will probably do until the end of the month. |
|
Concerns expressed by erlehmann on IRC:
|
|
Closing this for now; the best way to resolve the conflicts is to redo it. I don't quite see why this would cause the mentioned lag; the rendered textures should be cached. What has the resolution been set to? |
|
Care to redo it? |
sfan5
added
the
Adoption needed
Probably not, sorry. |
#10003 splitoff, code redundancy still needs to be fixed