GNOME Bugzilla – Bug 168890
Drivers are not yet modular
Last modified: 2009-05-16 12:01:58 UTC
Transports (like the one for CUPS) are modular. Drivers are not (ps2, pdf...). Other information:
Created attachment 38104 [details] Patch to make drivers in libgnomeprint modular This patch introduces modular drivers for libgnomeprint. It moves libgnomeprint/gnome-print-[ps2,pdf,...] to the subdirectories libgnomeprint/drivers: libgnomeprint drivers fax pdf postscript svg common (gnome-print-encode, gnome-pgl...) In my opinion, this reorganization makes the code much cleaner. Users can select during configure which drivers to compile (--without-drivers=postscript,pdf,svg). I implemented this by providing only one function gnome_print__context_get_type for each driver. Settings that each driver needs (paper width...) are implemented using the GObject property mechanism: When loading the driver, the call to gnome_print_config_configure_object scans the object for settings that are specified in the configuration file and passes them onto the object. In a second step, I'd like to move most parts of GnomePrintMeta into the drivers subdirectory, too.
Created attachment 38318 [details] [review] Modifications to configure.in and libgnomeprint/*.[c,h] This patch shows the modifications to configure.in (let users define which drivers and transports to compile) and the necessary modifications to libgnomeprint/*.[c,h], specifically gnome-print.c and gnome-print-[config,transport].c.
Created attachment 38319 [details] Replacement for subdirectory libgnomeprint/drivers
Created attachment 38320 [details] Replacement for subdirectory libgnomeprint/transports
Moving them will require CVS surgery, and while I like the idea of them being modular, I don't see a good reason to remove things like ps/pdf from the core. The use case for adding a module seems clear, but why would someone want to remove one of the base set ?
My main intention was to reduce the number of files in the libgnomeprint subdirectory. You just don't see which file belongs to which subsystem any more. Besides that, modular drivers make working on libgnomeprint (maintenance) much easier: - no need to recompile/install the whole library, - clean interfaces, - no need for differentiation in libgnomeprint-code between built-in and modular drivers...) The smaller the library (gnome-print) or driver, the easier it is for other people to get involved.
Comment on attachment 38319 [details] Replacement for subdirectory libgnomeprint/drivers We can not use this type of blob to do the reorg for two reasons. 1) It loses the CVS history of the files. We will need to do some surgery behind the scenes to make it work. 2) The files have changed since this was generated. Please replace this tarball with a patch for the files with any changes necessary, and a script to list of renamings.
Comment on attachment 38320 [details] Replacement for subdirectory libgnomeprint/transports similar problems to the driver tarball.
Comment on attachment 38318 [details] [review] Modifications to configure.in and libgnomeprint/*.[c,h] The --with-drivers / --with-transports extension to configure.in worry me, but may be useful. In GnomePrintConfig - a better name than 'ht' please. - Use g_hash_table_new_full and supply gt_foreach_func_free (with a better name) as the value handler. Then get rid of the call to g_hash_table_foreach in the finalize. gnome_print_config_translate_key : EEK! never ever write for (... i < strlen (s) ). That makes things O(n^2). All you are doing is a simple map just iterate on a char *. The whole hashtable of object values is interesting but I have no idea what the implementation is trying to do. Why do you keep an array of objects ? Assigning the same value to multiple objects that have the same property name seems odd. What are the gnome_print_context property get/set routines for ? why did you chose 'gnomeprint-{driver}' in place of the current 'gnome-print-{driver}' ?
I agree with the general principle. Once the win32 backend goes into cvs lets pick a time to do the cvs surgery.
Created attachment 39442 [details] [review] Minimal changes necessary for compilation after movement of files Save this file as CVS/libgnomeprint/reorg.diff. It contains the minimal changes necessary for compilation after movement of files using script (to follow).
Created attachment 39445 [details] Script to handle movement of files Save this script as CVS/libgnomeprint/reorg.sh. Run it: (1) It'll apply the CVS/libgnomeprint/reorg.diff (mostly fixes of includes). (2) It copies several files from libgnomeprint/... into libgnomeprint/drivers/... (3) It generates the necessary Makefiles in the libgnomeprint/drivers subdirectories. (4) It adjusts the path to the header files Afterwards, you should be able to compile libgnomeprint as before. Nothing has changed except the location of some files. Once the files are in the right location in CVS, I'll submit the patches to make the drivers modular. Jody, is this what you were looking for? I am not very good in scripting, but I hope the script is good enough to handle the movement of the files.
That's a bit more elaborate than necessary. I need to manually move (actually copy) the files on cvs.gnome.org so that we can preserve the development history. What would be really useful is a simple list with two columns <original file> <destination path> eg libgnomeprint/gnome-print-pdf.c libgnomeprint/drivers/pdf/gnome-print-pdf.c First I'll make the files available in their new locations. We can handle all other manipulations to actually use them as patches.
Here you are: libgnomeprint/art_rgb_affine_private.h libgnomeprint/drivers/common libgnomeprint/art_rgba_rgba_affine.[c,h] libgnomeprint/drivers/common libgnomeprint/art_rgba_svp.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-pgl-private.h libgnomeprint/drivers/common libgnomeprint/gnome-pgl.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-encode.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-rbuf.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-rgbp.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-frgba.[c,h] libgnomeprint/drivers/frgba libgnomeprint/gnome-print-pdf-private.h libgnomeprint/drivers/pdf libgnomeprint/gnome-print-pdf.[c,h] libgnomeprint/drivers/pdf libgnomeprint/gnome-print-pdf-t[t,1].c libgnomeprint/drivers/pdf libgnomeprint/gnome-print-ps2.[c,h] libgnomeprint/drivers/postscript libgnomeprint/gnome-print-svg.[c,h] libgnomeprint/drivers/svg libgnomeprint/drivers/gnome-print-fax.[c,h] libgnomeprint/drivers/fax libgnomeprint/drivers/gnome-print-fax-g3.h libgnomeprint/drivers/fax libgnomeprint/drivers/gnome-print-omni2.[c,h]pp libgnomeprint/drivers/omni2 I suggest you rename the last 2 files to gnome-print-omni2.[c,h] while you are at it. My suggestion for the transports subdirectory: gp-transport-lpr.[c,h] lpr/gnome-print-transport-lpr.[c,h] gp-transport-file.[c,h] file/gnome-print-transport-file.[c,h] gp-transport-custom.[c,h] custom/gnome-print-transport-custom.[c,h] ../modules/cups/gnome-print-cups-transport.c cups/gnome-print-transport-cups.c ../modules/papi/gnome-print-papi-transport.c papi/gnome-print-transport-papi.c ../modules/win32/gnome-print-win32-transport.c win32/gnome-print-transport-win32.c Like with the drivers subdirectory, I'd submit then 4 new files, transports/common/gnome-print-transport-fd.[c,h] transports/common/gnome-print-transport-pipe.[c,h] that would be used by the other transports, too. That would save us a lot of copy & paste.
Hello again! I realized that frgba is no driver itself - it is (currently) only used by the PostScript driver. Therefore, the frgba code should be moved to the common directory: libgnomeprint/art_rgb_affine_private.h libgnomeprint/drivers/common libgnomeprint/art_rgba_rgba_affine.[c,h] libgnomeprint/drivers/common libgnomeprint/art_rgba_svp.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-pgl-private.h libgnomeprint/drivers/common libgnomeprint/gnome-pgl.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-encode.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-rbuf.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-rgbp.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-frgba.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-pdf-private.h libgnomeprint/drivers/pdf libgnomeprint/gnome-print-pdf.[c,h] libgnomeprint/drivers/pdf libgnomeprint/gnome-print-pdf-t[t,1].c libgnomeprint/drivers/pdf libgnomeprint/gnome-print-ps2.[c,h] libgnomeprint/drivers/postscript libgnomeprint/gnome-print-svg.[c,h] libgnomeprint/drivers/svg libgnomeprint/drivers/gnome-print-fax.[c,h] libgnomeprint/drivers/fax libgnomeprint/drivers/gnome-print-fax-g3.h libgnomeprint/drivers/fax libgnomeprint/drivers/gnome-print-omni2.[c,h]pp libgnomeprint/drivers/omni2
(1) gdi is a driver, too. (2) gnome-[pgl,rfont] are used by the drivers, libgnomeprintui/gnome-canvas-hacktext and libgnomeprintui/gnome-font-dialog. The headers are published. Therefore, for now, we cannot make them private. That is: libgnomeprint/art_rgb_affine_private.h libgnomeprint/drivers/common libgnomeprint/art_rgba_rgba_affine.[c,h] libgnomeprint/drivers/common libgnomeprint/art_rgba_svp.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-encode.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-rbuf.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-rgbp.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-frgba.[c,h] libgnomeprint/drivers/common libgnomeprint/gnome-print-pdf-private.h libgnomeprint/drivers/pdf libgnomeprint/gnome-print-pdf.[c,h] libgnomeprint/drivers/pdf libgnomeprint/gnome-print-pdf-t[t,1].c libgnomeprint/drivers/pdf libgnomeprint/gnome-print-ps2.[c,h] libgnomeprint/drivers/postscript libgnomeprint/gnome-print-svg.[c,h] libgnomeprint/drivers/svg libgnomeprint/gnome-print-gdi.[c,h] libgnomeprint/drivers/gdi libgnomeprint/drivers/gnome-print-fax.[c,h] libgnomeprint/drivers/fax libgnomeprint/drivers/gnome-print-fax-g3.h libgnomeprint/drivers/fax libgnomeprint/drivers/gnome-print-omni2.[c,h]pp libgnomeprint/drivers/omni2
I wonder at the utility of the common directory. Why not leave those symbols in the library itself for use by mythical 3rd party / external drivers.
The answer is simple for art_rgb*: This belongs into libart_lgpl. If someone else would like to use it, we should publish it within libart_lgpl. gnome-print-encode is not very sophisticated, is currently private and only used by drivers -> therefore the suggestion to move it into the drivers subdirectory. rgbp, rbuf and frgba: You are right. But my goal is a small libgnomeprint. If someone develops a driver that needs one of those 3 files, why not developing this driver within libgnomeprint?
Closing.