Skip to content

Conversation

@jayaddison
Copy link
Contributor

@jayaddison jayaddison commented Mar 1, 2025

  • Remove implicit SIOD array accesses
  • Migrate removed drawable API calls to item equivalents
  • Introduce TinyScheme vector-ref and vector-length calls
  • Migrate from get-active-layer to get-selected-layers
  • Call file-png-export (instead of save) with named arguments
  • Remove the no-longer-used visibleStuff variable

These changes, in combination with #95 and some additional prerequisite install dependencies, allow me to use Debian testing (trixie) to rebuild what appear naively to be very similar (but not identical) PNG output images to those shipped in Debian stable (bookworm).

Resolves #94

 * Remove implicit SIOD array accesses[1]
 * Migrate removed `drawable` API calls to `item` equivalents
 * Introduce TinyScheme `vector-ref` and `vector-length` calls
 * Migrate[2] from `get-active-layer` to `get-selected-layers`
 * Call `file-png-export` (instead of `save`) with named arguments

Refs:
 - [1] https://www.gimp.org/docs/script-fu-update.html#carempty
 - [2] https://samjcreations.blogspot.com/2022/08/api-gimp-29912-script-fu.html
@jayaddison jayaddison marked this pull request as ready for review March 3, 2025 19:48
@jayaddison
Copy link
Contributor Author

I've tested a build of this branch plus #95 against the existing Debian v7.1 OpenGFX package --and the results are almost identical, and all seems fine in-game with the OpenGFX base set enabled. The only difference is in the ogfxe_extra.grf file -- and within that, only the NFO section (after grfcodec-decoding it) differs. I'm not sure exactly what the differences represent, or where in the game the relevant sprites would appear.

frosch123
frosch123 previously approved these changes Mar 17, 2025
Copy link
Member

@frosch123 frosch123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jayaddison.
I checked the nfo files, they just differ in the version/git-revision, which is included in them.

@jayaddison
Copy link
Contributor Author

I'm very glad to learn that that's the difference! - thank you @frosch123 (and also for the PR approval).

@matthijskooijman
Copy link
Contributor

Many thanks for picking this up and diving in so deep!

I looked through the diff and it looks ok to me (but I'm not familiar enough with lisp and script-fu to really tell if it is ok, at least the changes do not look wrong to me). Only remark I have is that I think the visibleStuff variable is now unused and could be removed? I see two mentions, both of which are assignments (which is a bit weird, so maybe I am misreading).

@matthijskooijman
Copy link
Contributor

I've also tested these changes (this PR rebased on top of master/#95) in the Debian build on a clean sid chroot, and that also seems to work well (with gimp3-rcsomething). I did see a lot of these warnings:

Traceback (most recent call last):                                                                                                                                                                      
  File "/usr/lib/x86_64-linux-gnu/gimp/3.0/extensions/org.gimp.extension.goat-exercises/goat-exercise-py3.py", line 16, in <module>
    import gi                                                                                                                                                                                           
ModuleNotFoundError: No module named 'gi'                                                                                                                                                               
/usr/bin/gimp-console: LibGimpBase-WARNING: gimp-console: gimp_wire_read(): unexpected EOF     

Which could be fixed by installing python3-gi, but they also seem harmless (the build works, so this is just for loading gimp plugins that are not actually used, I suspect). I won't add a build-dep for this, since if these are needed to run gimp-console, then it is up to the gimp package to depend on them.

These changes, in combination with #95 and some additional prerequisite install dependencies

@jayaddison you mentioned additional dependencies, which did you mean by that?

@jayaddison
Copy link
Contributor Author

I looked through the diff and it looks ok to me (but I'm not familiar enough with lisp and script-fu to really tell if it is ok, at least the changes do not look wrong to me). Only remark I have is that I think the visibleStuff variable is now unused and could be removed? I see two mentions, both of which are assignments (which is a bit weird, so maybe I am misreading).

Thanks @matthijskooijman for reviewing this - yep, I also think we can remove the visibleStuff variable -- I'll go ahead and do that in a few moments.

These changes, in combination with #95 and some additional prerequisite install dependencies

@jayaddison you mentioned additional dependencies, which did you mean by that?

Hm, good question - that wasn't very clear, in retrospect. I'm fairly sure I meant the regular build dependencies (e.g. apt build-dep ...) of the Debian package, though. Certainly that's what I used (without any additional dependencies required) during the more recent round of testing.

@matthijskooijman
Copy link
Contributor

I also tried to see if I could make these changes backward-compatible, but could not figure this out. I got stuck on the line (the first one I tried):

		(file-png-export #:run-mode RUN-NONINTERACTIVE #:image image #:file outImageName #:interlaced FALSE #:compression 9 #:bkgd FALSE #:offs FALSE #:phys FALSE #:time FALSE #:save-transparent FALSE)

This seems to use "uninterned symbols" (in lisp) or keywords (scheme) created by the #: operator, but I suspect that the lisp version in gimp2 does not support these somehow, so this fails to compile with:

ts> Error: undefined sharp expression 
ts> Error: eval: unbound variable: :run-mode 

This happens even when I use an if to not evaluate these things, so it just fails to parse.

I tried replacing them with ordinary symbols ('run-mode') or strings, which are not accepted by file-png-export. I thought they could maybe be created by make-symbol`, but that is also not available/defined (also not in gimp3). So I'm out of options here.

What we could of course do is keep two versions of scripts/gimpscript (or keep one with some post-processing), and then just select the right one from the Makefile (since we're post-processing the script into an .scm file anyway)? I was slightly worried about bitrot in the gimp2 script if more changes would be made, but OTOH I see that the script has seen just one whitespace change since it was added in 2011, so maybe it will stay working just fine...

Would that make sense?

@jayaddison
Copy link
Contributor Author

What we could of course do is keep two versions of scripts/gimpscript (or keep one with some post-processing), and then just select the right one from the Makefile (since we're post-processing the script into an .scm file anyway)? I was slightly worried about bitrot in the gimp2 script if more changes would be made, but OTOH I see that the script has seen just one whitespace change since it was added in 2011, so maybe it will stay working just fine...

Would that make sense?

Although it's slightly less neat, that seems like a good idea to me; allowing other systems/distros to upgrade at their own pace.

@jayaddison
Copy link
Contributor Author

@matthijskooijman should I submit #95 and/or the changes here up-to-and-including 0e0bf94 as patches for the Debian package in trixie, regardless of the backwards-compatibility check?

(I'm usually a bit reluctant to send patches downstream until code is merged; in this case I hope we're relatively confident now that the result is a no-op... and also I get the sense it might take me a while to get around to the backcompat code, even if it seems like it should be a small thing)

@jayaddison
Copy link
Contributor Author

(and when I say backcompat code.. -- I mean possibly the tweaks to the Makefile, as suggested.. but in practice, maybe I'll feel that it's worth attempting a script-fu based alternative, to compare and determine which approach seems more maintainable. but I'm procrastinating over this)

@frosch123
Copy link
Member

@jayaddison @matthijskooijman
I do not understand what you mean with "backcompat code".
I tested this with gimp-console 2.10.34, and it works for me. Am I missing something?

@matthijskooijman
Copy link
Contributor

Hm, that is interesting. I get:

# gimp-console -version
GNU Image Manipulation Program version 2.10.34
# gimp-console -n -i --batch-interpreter=plug-in-script-fu-eval -b - <sprites/png/trees/arctic/tree_06_snow_leaf.gimp.scm                   
Welcome to TinyScheme, Version 1.40                                                                                                                                                                          
Copyright (c) Dimitrios Souflis                                                                                                                                                                              
                                                                                                                                                                                                             
ts> script-fu-set-all-layers-invisible                                                                                                                                                                       
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :run-mode                                                                                                                                                                 
                                                                                                                                                                                                             
ts> 1                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :image                                                                                                                                                                    
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: image                                                                                                                                                                     
                                                                                                                                                                                                             
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :file                                                                                                                                                                     
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: outImageName                                                                                                                                                              
                                                                                                                                                                                                             
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :interlaced                                                                                                                                                               
                                                                                                                                                                                                             
ts> 0                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :compression                                                                                                                                                              
                                                                                                                                                                                                             
ts> 9                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :bkgd                                                                                                                                                                     
                                                                                                                                                                                                             
ts> 0                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :offs                                                                                                                                                                     
                                                                                                                                                                                                             
ts> 0                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :phys                                                                                                                                                                     
                                                                                                                                                                                                             
ts> 0                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :time                                                                                                                                                                     
                                                                                                                                                                                                             
ts> 0                                                                                                                                                                                                        
ts> Error: undefined sharp expression                                                                                                                                                                        
                                                                                                                                                                                                             
ts> Error: eval: unbound variable: :save-transparent                                                                                                                                                         
                                                                                             

(and then some more)

Are you sure you're testing the right code and the right gimp-console version?

and also I get the sense it might take me a while to get around to the backcompat code, even if it seems like it should be a small thing

Maybe I'll try to whip up something sooner then - I'd rather backport merged code, or even get a new release if possible in the next couple of weeks (Debian freeze deadline is April 15th or so).

@frosch123
Copy link
Member

@matthijskooijman
Ah, the Makefile is redirecting all gimp-console output to /dev/null.
Without that I also get those messages, but it works anyway and creates a correct result?

@matthijskooijman
Copy link
Contributor

Without that I also get those messages, but it works anyway and creates a correct result?

No, it does not generate any pngs, but just uses the ones in the repo. Try make clean-gfx to throw those away, then you'll see no new pngs are generated.

@matthijskooijman
Copy link
Contributor

I've implemented the Makefile-based gimp2 compatibility, seems to work fine. I've created #98 with this to replace this PR.

@jayaddison
Copy link
Contributor Author

Thanks @matthijskooijman; closing this pull request as superseded by #98.

@jayaddison jayaddison closed this Mar 24, 2025
@jayaddison jayaddison deleted the issue-94/gimp3-scriptfu-compatibility branch March 24, 2025 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gimpscript: GIMP3 compatibility problem(s)

3 participants