Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
My first python plugin review
#11
I've updated the code to handle pretty much any aspect ratio, and I added some standard aspect ratios as defaults.

https://github.com/origamifreak2/Gimp-Pl...t_ratio.py

I actually took a look at one of your plugins @Ofnuts to see how to register multiple plugins under the same submenu. Let me know if there's anything I can improve  Smile
Reply
#12
(01-01-2025, 12:39 AM)origamifreak2 Wrote: Thanks Ofnuts  Smile  . I updated my code. I also added some logic to convert the image to RGB mode if it's indexed.

This isn't something I would do. I would only enable the filter on RGB* and GRAYSCALE*. Then if someone really wants to use the filter on an indexed image, de-indexing it beforehand  is a quick single-click away. Because now your filter needs to re-index the image, and how to do so properly is a nice can of worms (how many colors? which palette?).
Reply
#13
Thanks again Ofnuts

I took your suggestion. I've also made a ton of other changes to the code.
  • added ability to fill background with the active foreground color
  • added ability to fill background with the active background color
  • added ability to fill background with a different user selected color
  • added ability to fill background with transparency
  • added ability for user to select a specific blur radius between 0 and 500
The code is quite a bit more complicated now, but please feel free to take a look and let me know if anything could be done better.
Reply
#14
elif(background_type == 1 or background_type == 2 or background_type == 3) better written as if background_type in [1,2,3]

elif(background_type == 4): # transparent background
# do nothing since resized canvas background is already transparent
pass


better written as
else: # transparent background case: do nothing since resized canvas background is already transparent
pass

(always better to have an explicit else to catch what we could have forgotten)

Instead of saving the color, you can bracket your code with gimp.context_push()/gimp.context_pop() (like the undo group), so that any context change you make for the script is reset when the script exits.

Blur radius can be a PF_SLIDER, easier for the user and since you add value boundaries you naturally limit the user to what Gimp's blur allows.

When you use pre-defined values as parameters, use their name, for instance pdb.gimp_image_merge_down(image, drawable, 1) is better written pdb.gimp_image_merge_down(image, drawable,CLIP_TO_IMAGE ) (note the "-/_" substitution).
Reply
#15
I believe I've implemented all your suggestions @Ofnuts. Thanks again. Let me know if there's anything else that could be improved.
Reply
#16
The idea of context_push()/Context_pop() is that they bracket the whole code (like the undo_group thing) , so that once you have set it up your code can alter the context without having to worry about it.

Beyond this it becomes a matter of style. Personally, I loathe if/else, because this tends to create code duplicates(*) so you have to maintain two or more branches in parallel. So my way of writing this:

Code:
       if(background_type == 1): # foreground color
           pdb.gimp_edit_bucket_fill(background, BUCKET_FILL_FG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1)
       elif(background_type == 2): # background color
           pdb.gimp_edit_bucket_fill(background, BUCKET_FILL_BG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1)
       elif(background_type == 3): # other color
           pdb.gimp_context_push()
           pdb.gimp_context_set_background(background_color) # set new background color
           pdb.gimp_edit_bucket_fill(background, BUCKET_FILL_BG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1) # fill background with new color
           pdb.gimp_context_pop()

would be:

Code:
       gimp.context_push()

       # most of your code goes here

       color=[gimp.get_foreground(),gimp.get_background(),background_color][background_type-1]
        pdb.gimp_edit_bucket_fill(color, BUCKET_FILL_BG, LAYER_MODE_NORMAL_LEGACY, 100, 0, False, 1, 1) # fill background with new color

       # some more of your code goes here

       gimp.context_pop()

Then you discover that there are better ways to fill the layer, so you need only one change
Code:
       gimp.context_push()

       # most of your code goes here

        color=[gimp.get_foreground(),gimp.get_background(),background_color][background_type-1]
       gimp.set_foreground(color)
       layer.fill(FILL_FOREGROUND)

       # some more of your code goes here

       gimp.context_pop()

(I tend to use the object methods instead of the pdb.* procedures when they exists, this make the code easier to read. To find what they are, do a dir(gimp), dir(image, or dir(layer) in the python console (after initializing image and layer of course) . The mapping are usually quite obvious.

(*) this is called the DRY principle: "don't repeat yourself". The opposite is of course the WET principle: "We enjoy typing"
Reply


Forum Jump: