GNOME Bugzilla – Bug 315130
PDB interface to copy/cut/paste named
Last modified: 2005-09-27 09:16:37 UTC
My plugin needs to copy/paste, and it would be much better if it could do it without clobbering the existing clipboard content. To be specific: my '24bit backbuffer simulation' plugin exploits the conversion abilities of paste; it keeps a temporary indexed image to paste areas into for indexization. It then copies them and pastes them back into the current image to finish. The only workable workaround I know is a bit ugly and slow: * paste the selection on a new layer. after pasting the indexized selection back, copy this layer and destroy it.
It's actually a bug if important core functionality is missing in the PDB. Moreover it's trivial to implement.
.. I'm lost as to how to do that. copy/cut/paste named seem to be implemented entirely in app/actions/edit-commands.c. Would i need to move the guts of them out into a file in libgimp/ and then add pdb entries wrapping them in tools/pdbgen/pdb/edit.pdb?
Um, no the functionality would go to app/core/gimp-edit.c, and actually I'm almost finished hacking this... Your report didn't sound as if you were about to implement this, sorry for being fast this time :)
Fixed in CVS: 2005-09-03 Michael Natterer <mitch@gimp.org> * app/core/gimp-edit.[ch] (gimp_edit_paste_as_new): don't create a display here. (gimp_edit_named_cut) (gimp_edit_named_copy) (gimp_edit_named_copy_visible): new functions containing named buffer wrappers around the functions affecting the global buffer only. * app/actions/edit-commands.c: use the new functions instead of implementing them here, create a display for the image returned by paste as new. * app/actions/buffers-commands.c * app/widgets/gimptoolbox-dnd.c: create displays here too. * tools/pdbgen/pdb/edit.pdb: added wrappers for paste as new and wrappers for all the cut/copy/paste named stuff. Fixes bug #315130. Cleaned up and de-obfuscated. * libgimp/gimp.def: changed accordingly. * app/pdb/edit_cmds.c * app/pdb/internal_procs.c * libgimp/gimpedit_pdb.[ch]: regenerated.
There are still a few things missing that would make this work best: * delete-named-buffer PDB entry (otherwise, plugins will create more and more duplicate named buffers as they are rerun.) * 'copy visible named' menu item. I'll look into it.
Right, reopening.
Created attachment 51779 [details] [review] fixes the mentioned shortcomings This patch implements: * PDB entry 'gimp-buffer-delete' * 'edit->buffer->copy visible named' menu item. Check line #170 -- Do I need to unref there, after removing the buffer from its container? That's what i do currently. This was surprisingly easy to do :) Touched files: app/actions/edit-actions.c app/actions/edit-commands.[ch] menus/image-menu.xml.in tools/pdbgen/pdb/edit.pdb app/pdb/edit_cmds.c libgimp/gimpedit_pdb.[ch]
Thanks for the patch, some comments: - you must not unref after removing from the container. - the new pdb function doesn't live in its file's namespace. - your patch breaks indentation all over the place. Please fix these issues so the patch can be comitted.
Created attachment 51782 [details] [review] adds 'copy visible named' to menus Fixed coding style issues. Tried to fix other issues, see following file.
Created attachment 51783 [details] buffers.pdb - implements gimp-buffer-delete, if pdbgen understood it. I based this off the smallest .pdb file, help.pdb. It causes pdbgen to fail trying to match a regexp , though it seems virtually identical in every aspect. What am I doing wrong here?
Created attachment 51790 [details] implements gimp-buffer-delete This file comprises : * buffer-delete-cmd.diff (implements all the changes that do not create new files) * app/pdb/buffers_cmds.c * libgimp/gimpbuffers_pdb.[ch] * tools/pdbgen/pdb/buffers.pdb Done this way cause i apparently need write-access in order to add files. (unless there is a option to CVS diff which causes it to generate patches adding 'unknown' files?) Anyway, this works and fixes everything you mentioned.
The attached menu item patch is somehow broken. I applied it manually anyway: 2005-09-05 Michael Natterer <mitch@gimp.org> * app/actions/edit-actions.c * app/actions/edit-commands.[ch] * menus/image-menu.xml.in: applied modified patch from David Gowers which adds an "edit-named-copy-visible" actions and its menu item. Addresses bug #315130. Unfortunately I replied in a hurry before and you misunderstood me :/ I didn't mean that delete buffer should get its own PDB group, I was rather thinking of renaming it to gimp-edit-named-delete. IMHO this single functions doesn't justify adding another group (unless we find other useful functionality that should go in there).
Btw, you don't need to attach the autogenerated files.
Created attachment 51804 [details] [review] diff implementing 'gimp-edit-named-delete' PDB func Okay, done.
Applied to CVS: 2005-09-05 Michael Natterer <mitch@gimp.org> * tools/pdbgen/pdb/edit.pdb: applied patch from David Gowers which adds "gimp-edit-named-delete". Addresses bug #315130. Added "gimp-edit-named-rename" additionally. * libgimp/gimp.def: changed accordingly. * app/pdb/edit_cmds.c * app/pdb/internal_procs.c * libgimp/gimpedit_pdb.[ch]: regenerated. Also added "gimp-edit-named-rename". I wonder if your previous patch with the separate buffer.pdb file wouldn't be the right thing to do after all... There is for example no way of getting a buffers size, image type etc., all things that are needed to use the new buffer API in a straightforward way. What do you think?
Okay.. So one more function, gimp-buffer-get-info, returning (w,h,type); plus renaming and moving the two recently-added functions?. Is type derived from bpp? (?) Then how do you tell Indexed/Grey apart?
I don't really follow your last comment, esp. the last line...
One more function is needed, in addition to moving named-delete and named-rename into buffers.pdb, if this change is made. It seems good to me (I prefer gimp-buffer-* over gimp-edit-named-*.). Whatever is decided, i can make a patch for it pretty quickly. by "Is type derived from bpp? (?) Then how do you tell Indexed/Grey apart?" I mean, tile managers don't have knowledge of their type, just bpp, right? (Indexed and Grey) and (IndexedA and GreyA) are therefore indistinguishable. It's probably not an issue, as I now realize GIMP probably only keeps Buffers of RGB(A) or Grey(A) format, and converts indexed(a)->RGB(a) on copy. Is that correct?
Buffers can't be indexed, so a bpp->type mapping look as follows: 1 -> GRAY 2 -> GRAYA 3 -> RGB 4 -> RGBA That's ugly, but the uglyness is internal ;) Also, the file should be called buffer.pdb, not buffers.pdb (just ad brush.pdb, pattern.pdb etc.) and the copy/paste functions should stay in edit.pdb
Fixed in CVS: 2005-09-27 Michael Natterer <mitch@gimp.org> * app/core/gimpbuffer.[ch]: added gimp_buffer_get_bytes() and gimp_buffer_get_image_type(). * tools/pdbgen/pdb/edit.pdb: removed edit_named_rename() and edit_named_delete(). * tools/pdbgen/Makefile.am * tools/pdbgen/groups.pl * tools/pdbgen/pdb/buffer.pdb: new PDB group featuring buffer_rename(), delete(), get_width(), get_height(), get_bytes(), get_image_type(). Fixes bug #315130. * app/pdb/Makefile.am * libgimp/Makefile.am * libgimp/gimp.def: changed accordingly. * app/pdb/buffer_cmds.c * app/pdb/edit_cmds.c * app/pdb/internal_procs.c * libgimp/gimp_pdb.h * libgimp/gimpbuffer_pdb.[ch] * libgimp/gimpedit_pdb.[ch]: (re)generated.