smcameron / gaseous-giganticus

This program procedurally generates gas giant cubemap textures for the game Space Nerds In Space. https://www.patreon.com/smcameron
https://smcameron.github.io/space-nerds-in-space/gaseous-giganticus-slides/slideshow.html#1
GNU General Public License v2.0
109 stars 7 forks source link

--output does not respect the user provided output_file_prefix #11

Closed WickedSmoke closed 10 months ago

WickedSmoke commented 10 months ago

save_output_images() prepends a period character into the PNG path making output options such as "-o /tmp/out-" impossible to use. Here's a patch:

diff --git a/gaseous-giganticus.c b/gaseous-giganticus.c
index 87aa80c..1c659fc 100644
--- a/gaseous-giganticus.c
+++ b/gaseous-giganticus.c
@@ -1232,5 +1232,5 @@ static void save_output_images(char *output_file_prefix, int sequence_number, un
                if (sequence_number < 0)
-                       sprintf(filename[i], ".%s%d.png", output_file_prefix, i);
+                       sprintf(filename[i], "%s%d.png", output_file_prefix, i);
                else
-                       sprintf(filename[i], ".%s%04d-%d.png", output_file_prefix, i, sequence_number);
+                       sprintf(filename[i], "%s%04d-%d.png", output_file_prefix, i, sequence_number);
                if (png_utils_write_png_image(filename[i], image[i], DIM, DIM, has_alpha, 0)) {
smcameron commented 10 months ago

So the reason it uses a leading "." is because later, all 6 images get renamed at once without the ".". The reason it does that is because some programs monitor the files for changes as they are generated (mesh_viewer in space-nerds-in-space, for example), and if the files aren't renamed all at once, it will attempt to load all the files before they're finished generating. I think your fix will break that behavior (see commit f85a618aa32c7a03d3522dd21426fd6838b92a93).

I did not anticipate using this filename prefix to move the output to some other directory.

I'll think about how to fix it so that both these behaviors can work.

WickedSmoke commented 10 months ago

I was just kicking the tires and always put throw away data into the RAM disk (/tmp). In every other program I can think of output options are paths, so it was most unwelcome to get errors instead.

    0 /  1000 Saving Imagesfopen: ./tmp/out0.png:No such file or directory
Failed to write ./tmp/out0.png
fopen: ./tmp/out1.png:No such file or directory
Failed to write ./tmp/out1.png

Isn't there still a race condition with the renaming of multiple files? In my last project with cube maps I used a single atlas containing all six images.

smcameron commented 10 months ago

Isn't there still a race condition with the renaming of multiple files?

Yes, but in practice it seems not to be a problem, and when it was a problem, it's only a cosmetic problem.

I agree with you that the current behavior is a bug, and I'll try to fix it, just not this instant, maybe tomorrow.

smcameron commented 10 months ago

I believe this commit should fix it: 6612271cd063f5658140019f397952795ba32873

WickedSmoke commented 10 months ago

Being able to see the temp. files in the directory is much better to understand what the program is doing. Thanks for the fix.