Match failure.

Generic Texture Transfer

Reporter: RobertBColton  |  Status: closed  |  Last Modified: May 15, 2019, 10:37:23 AM

This one should hopefully be very obvious. I have been wanting to clean up some of the graphics texture copying functions because they duplicate a lot of logic. The obvious idea here is to just create a new set of abstractions that are even lower level and then rewrite the duplicate graphics texture functions with them. This will ensure that all of the graphics backends have more consistent and predictable behavior when it comes to the user functions that use these abstractions.

An important distinction I should make here is that this new API is based on the semantics of OpenGL and not Direct3D. Direct3D is actually lower level, allowing you to map the texture directly to system memory while you operate on its data in situ. So the Direct3D systems will implement the new API by performing an extra copy to system memory. This means the client is responsible for deleting the allocated memory regardless of the backend. The end result should be that Direct3D and OpenGL will have pretty similar performance when calling these functions.

These new functions are now mandatory for each backend to implement.

unsigned char* graphics_copy_texture_pixels(int texture, unsigned* fullwidth, unsigned* fullheight);
unsigned char* graphics_copy_texture_pixels(int texture, int x, int y, int width, int height);
void graphics_push_texture_pixels(int texture, int x, int y, int width, int height, unsigned char* pxdata);
void graphics_push_texture_pixels(int texture, int width, int height, unsigned char* pxdata);

These old mandatory functions were moved to General/ and reimplemented with the new abstract ones.

int graphics_duplicate_texture(int tex, bool mipmap=false);
void graphics_replace_texture_alpha_from_texture(int tex, int copy_tex);
void graphics_copy_texture(int source, int destination, int x, int y);
void graphics_copy_texture_part(int source, int destination, int xoff, int yoff, int w, int h, int x, int y);

I also prepared a benchmark to see if my concerns about DMA are valid. They don't seem to be as D3D9 got faster. I realized GL_RGBA is faster than 4 for the internal format of glTexImage2D. That is why GL3 was faster than GL1. I noticed no change in the performance of other benchmarks.

Download Benchmark GMK: texture-copy-benchmark.zip

var start;
start = get_timer();
repeat (50) {
	background_duplicate(bkg_0);
}
show_message(string(get_timer() - start));

Timing data recorded in microseconds.

codecov[bot]  
>Codecov Report

Merging #1648 into master will increase coverage by 0.21%.
The diff coverage is 37.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1648      +/-   ##
==========================================
+ Coverage   23.18%   23.39%   +0.21%     
==========================================
  Files         166      166              
  Lines       16664    16646      -18     
==========================================
+ Hits         3863     3894      +31     
+ Misses      12801    12752      -49
GL1 GL3 D3D9
Branch 02739 ✔ 2499 10476 ✔
Master 25215 2219 15261
Impacted Files Coverage Δ
...em/SHELL/Universal_System/Resources/fontstruct.cpp 1.17% <0%> (ø) ⬆️
...LL/Universal_System/Resources/backgroundstruct.cpp 41.44% <0%> (-0.28%) ⬇️
.../SHELL/Universal_System/Resources/spritestruct.cpp 73.91% <100%> (+1.9%) ⬆️
...stem/SHELL/Graphics_Systems/General/GStextures.cpp 19.26% <14.28%> (-3.69%) ⬇️
...stem/SHELL/Graphics_Systems/OpenGL1/GLtextures.cpp 54.9% <71.42%> (+16.19%) ⬆️
ENIGMAsystem/SHELL/Universal_System/fileio.cpp 83.33% <0%> (+0.79%) ⬆️
...system/SHELL/Graphics_Systems/General/GSsprite.cpp 9.77% <0%> (+2.22%) ⬆️
...stem/Extensions/DataStructures/data_structures.cpp 50.92% <0%> (+2.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 088bbcb...e3a20a0. Read the comment docs.

EnigmaBot  

UNMATCHED: Regression tests have indicated that graphical changes have been introduced. Carefully review the following image comparison for anomalies and adjust the changeset accordingly.

e3a20a0 Master Diff
Image Diff Image Diff Screen Save

RobertBColton  

EnigmaBot is correct that I've introduced graphical changes to that test. Since I now have sprite_duplicate working everywhere, including Direct3D, I would like to keep it that way.
Please sign in to post comments, or you can view this issue on GitHub.