After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 164539 - new PDB API needed for text tool
new PDB API needed for text tool
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: libgimp
git master
Other All
: Normal enhancement
: 2.6
Assigned To: GIMP Bugs
GIMP Bugs
: 345994 506737 (view as bug list)
Depends on:
Blocks: 101604 136740 151686 167260 314777
 
 
Reported: 2005-01-19 00:13 UTC by Sven Neumann
Modified: 2008-04-05 15:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed text_layer.pdb (perl source) (23.83 KB, text/plain)
2006-06-12 18:58 UTC, weskaggs
Details
resulting libgimp/gimptextlayer_pdb.h file (6.16 KB, text/plain)
2006-06-12 18:59 UTC, weskaggs
Details
New text_layer API PDB design for gimp-2.6 (43.52 KB, application/x-palm-database)
2008-03-31 10:45 UTC, heese
Details
reviewed text layer PDB design (33.53 KB, text/plain)
2008-03-31 21:45 UTC, Sven Neumann
Details
another iteration of the text layer API (33.68 KB, text/plain)
2008-03-31 22:03 UTC, Sven Neumann
Details

Description Sven Neumann 2005-01-19 00:13:45 UTC
The only way to control creation of text is by use of the old text PDB API. This
API is just a compat wrapper and doesn't make any of the new text tool features
available. There's also no way to detect a text layer, nor to get any
information from it, let alone to modify the text.
Comment 1 weskaggs 2005-01-19 17:49:13 UTC
If you make a list of the functions that are needed, I'll implement them.  (I
have already sketched out some of the basic ones.)  The only completely obvious
one is gimp_layer_is_text().  Of course there also need to be text_new(),
get_text(), and set_text() functions, but the correct namespace for these is not
quite so obvious.
Comment 2 Sven Neumann 2005-01-20 20:43:06 UTC
This report isn't actually a request for help, it's a bug report to remind me
that we (a) need to come up with a reasonable API and (b) that I need to
implement thsi API for GIMP 2.4.
Comment 3 weskaggs 2006-05-20 23:27:25 UTC
Bumping to Future because this will not be addressed for 2.4.
Comment 4 weskaggs 2006-06-12 18:56:45 UTC
Well, thinking that this is really desirable for 2.4, I have put together a basic text layer api.  I will attach the text_layer.pdb file I wrote, and, for convenience in just looking at the resulting api, the gimptextlayer_pdb.h file that is produced.  I have tested the "getters" and they work, except maybe the one for color, which doesn't seem to be working but I can't figure out why.  I have tested some of the setters as well.  Note that this requires the enums in text-enums.h to be exported in order to build.  (I am aware that not all of the variables in the api are functional right now.)

Comment 5 weskaggs 2006-06-12 18:58:14 UTC
Created attachment 67208 [details]
proposed text_layer.pdb (perl source)
Comment 6 weskaggs 2006-06-12 18:59:31 UTC
Created attachment 67209 [details]
resulting libgimp/gimptextlayer_pdb.h file
Comment 7 weskaggs 2006-06-16 18:24:16 UTC
Targetting to 2.4 and setting severity to blocker to make sure this is looked at.  If the proposal is too far off the mark, this can be bumped.
Comment 8 Sven Neumann 2006-06-27 07:31:41 UTC
*** Bug 345994 has been marked as a duplicate of this bug. ***
Comment 9 Sven Neumann 2006-06-27 07:32:46 UTC
Postponing to 2.6 so that we can take the time to review the API and come up with something solid.
Comment 10 Sven Neumann 2008-01-01 22:43:49 UTC
*** Bug 506737 has been marked as a duplicate of this bug. ***
Comment 11 heese 2008-03-31 10:45:46 UTC
Created attachment 108321 [details]
New text_layer API PDB design for gimp-2.6

This is now my proposal for the new design for the PDB. It includes the latest changes that Sven told me. Although I have only commented the "text_layer_get_coordinates" section, as it needs further discussion. If the GSoC project for the on-canvas text editing succeeds in changing to pangocairo (and I guess that will be so :) ), this code needs a few changes anyway.
Comment 12 Kevin Cozens 2008-03-31 16:44:10 UTC
Due to the size of the file and the mixing in of code I had to grep for lines starting with sub to get a better sense of what is proposed. The text API is looking quite good at this point. Most of the basic (and some not so basic) features are available.

Are there plans to allow use of Pango markup as well as support for some of the other attributes listed in the "Text Attribute Markup" page of Pango? Being able to use Pango markup via a call such as text_layer_set_text_markup (or ..._from_markup) would add a lot of flexibility and control over the appearance of text in a text layer.

I am not sure about "text_layer_discard_text". The name makes it sound like a method to remove the text from a text layer. Perhaps a better name would be of the form "text_layer_convert_to..." or "text_layer_make_normal_layer".
Comment 13 Sven Neumann 2008-03-31 17:55:05 UTC
I will have to review your patch. It exports some functions which should definitely not be exported to the PDB. gimp_text_layer_discard_text() for example is completely internal and there is absolutely no need this function should ever be called from a plug-in. Same for gimp_text_layer_get_coordinates().
Comment 14 weskaggs 2008-03-31 18:45:20 UTC
I would say there probably ought to be a way for plug-ins/scripts to access the locations of individual characters, but since it is always possible to add new API, but impossible to get rid of old API, it's best to be conservative and think it through carefully.

Regarding markup, the necessary internal support for it doesn't exist yet in Gimp, and adding it will be nontrivial.

In any case, it would be nice to get this in reasonably soon, because it would enable rewriting at least some of the logo scripts to use this functionality instead of the rather kludgy approach they currently use.
Comment 15 Sven Neumann 2008-03-31 21:45:59 UTC
Created attachment 108372 [details]
reviewed text layer PDB design

I am attaching a reviewed and massively changed version of the PDB file. This is untested and more work is needed here, but it should get us one step closer. I am just attaching this here, so I doesn't get lost. Will continue to work on it some more over the next days.
Comment 16 Sven Neumann 2008-03-31 21:48:03 UTC
2008-03-31  Sven Neumann  <sven@gimp.org>

	* app/text/text-enums.[ch]
	* libgimpbase/gimpbaseenums.[ch]: as a first step towards a new
	text PDB API, moved GimpTextDirection and GimpTextJustification
	enums to libgimpbase.

	* libgimpbase/gimpbase.def: updated.
Comment 17 Sven Neumann 2008-03-31 22:03:01 UTC
Created attachment 108375 [details]
another iteration of the text layer API
Comment 18 weskaggs 2008-03-31 23:11:49 UTC
I only spot a couple of things that look questionable on a first pass:

1) Should it be "font_size" instead of "fontsize"?

2) Things that set "success" to FALSE should set "error" as well -- this can however be changed later, since it doesn't affect the API strictly.

3) I wonder whether it is appropriate to limit sizes to 8192, if the units can be points as well as pixels.  Suppose I am working with an image downloaded from Google Maps, and have set the resolution accordingly, to something like 100 m/pixel, say.  And suppose I want to use a plug-in or script to add labels on the basis of GPS coordinates.  A font size of 8192 pts would be far smaller than one pixel.  I'm not sure this is a realistic scenario, but it seems at least theoretically possible.
Comment 19 heese 2008-04-01 09:38:05 UTC
to 1)
As Sven has changed "letterspacing" to "letter_spacing" and "linespacing"  to "line_spacing", it would just be consequent to change "fontsize" to "font_size", too.

to 2)
As I have written the file, Sven told me that such a feature is planned, but not available yet. I would have set an "error" as well then...

to 3)
Personally, I don't see any advantage in limiting the font_size to 8192, except that it perhaps avoids a bug ;)

to the "gimp_text_layer_get_coordinates()":
I would like to see some function like this in the API, as I would make heavily use of it in my scripts. In fact the whole proposal would not be very useful to me if I don't get such a function :) ... Although I see that it shouldn't be like this in the PDB, perhaps some get_coordinates() code in the internal text API would be good, so that one could call some function from the PDB. Perhaps such a function could be added while porting to pangocairo!
Comment 20 Sven Neumann 2008-04-01 10:46:57 UTC
Guys, I said I am only attaching the file here so it isn't lost. I will continue to work on it. No need to comment on work in progress...
Comment 21 Sven Neumann 2008-04-02 19:30:12 UTC
Another small step ahead:

2008-04-02  Sven Neumann  <sven@gimp.org>

	* tools/pdbgen/stddefs.pdb: added shortcut for contributions from 
	Marcus Heese.

	* tools/pdbgen/pdb/drawable.pdb: added gimp_drawable_is_text_layer(),
	taken from the patch attached to bug #164539.

	* app/pdb/internal_procs.c
	* app/pdb/drawable_cmds.c
	* libgimp/gimpdrawable_pdb.[ch]: regenerated.

	* libgimp/gimp.def: updated.
Comment 22 Sven Neumann 2008-04-02 20:36:00 UTC
2008-04-02  Sven Neumann  <sven@gimp.org>

	* app/pdb/gimppdb-utils.[ch]: added gimp_pdb_layer_is_text_layer().
Comment 23 Sven Neumann 2008-04-02 20:56:15 UTC
With this commit the next text API lands in SVN:

2008-04-02  Sven Neumann  <sven@gimp.org>

	* tools/pdbgen/Makefile.am
	* tools/pdbgen/pdb/text_layer.pdb: new text layer PDB API created
	by Marcus Heese (see bug #164539).

	* app/pdb/Makefile.am
	* app/pdb/text_layer_cmds.c: new generated file.

	* libgimp/Makefile.am
	* libgimp/gimptextlayer_pdb.[ch]: new generated files.

	* app/pdb/internal_procs.[ch]
	* libgimp/gimp_pdb.h
	* tools/pdbgen/groups.pl: regenerated.

What's missing is the function to create a vectors object from a text layer. I will look into adding that next. Then this bug can be closed. Marcus, thanks a lot for your help with this.
Comment 24 Sven Neumann 2008-04-02 21:04:17 UTC
I would very much appreciate if someone could check that all functions work as expected and that the new API is sane and consistent.
Comment 25 weskaggs 2008-04-03 15:28:27 UTC
I wrote a little plug-in to exercise all the getters and setters, and found that they all appear to work correctly, with one exception.  (I didn't actually test the setters for language or text direction, but I tested all the others.)  The exception is the getter for color, which I might not be using correctly.  I used the code:

GimpRGB color;

  gimp_text_layer_get_color (layer_ID, &color);

  g_print ("%lg %lg %lg\n", color.r, color.g, color.b);

This code does not print the correct color.  The "set_color" function
did work correctly when I used it in a similar way.
Comment 26 Sven Neumann 2008-04-04 14:51:50 UTC
Added the PDB function to create a vectors object from a text layer:

2008-04-04  Sven Neumann  <sven@gimp.org>

	* tools/pdbgen/pdb/vectors.pdb: added
	gimp_vectors_new_from_text_layer().

	* app/pdb/internal-procs.c
	* app/pdb/vectors-cmds.c
	* libgimp/gimpvectors_pdb.[ch]: regenerated.

	* libgimp/gimp.def: updated.
Comment 27 Michael Natterer 2008-04-04 15:44:48 UTC
This fixes get_color() and brings the new functions into a sane order:

2008-04-04  Michael Natterer  <mitch@gimp.org>

	* tools/pdbgen/pdb/text_layer.pdb: reorder functions so getters
	and setters are together, rename fontsize() functions to
	font_size(), fix get_color() implementation.

	* app/pdb/text-layer-cmds.c
	* libgimp/gimptextlayer_pdb.[ch]: regenerated.

	* libgimp/gimp.def: changed accordingly.
Comment 28 Sven Neumann 2008-04-05 15:47:32 UTC
I'd say we can close this as FIXED now. We will want to extend the text API when the new text tool features have been finalized, but this can be dealt with in another bug report.