Created attachment 37795 [details] [review]
proposed patch

- Add support of delayed rendering on Win32

- gtk_clipboard_request_targets() returns all available formats including those
registered by gtk+ apps

- image selection conversion are now bi-directional. Formats such as
"image/jpg", "image/png", etc. will be delayed-rendered to DIB by using
gdk-pixbuf, i.e. paint.exe can now paste what gimp.exe copied.

- add support of writing bmp to gdk-pixbuf

- fix a bug of the .ico writer- Add support of delayed rendering on Win32

- gtk_clipboard_request_targets() returns all available formats including those
registered by gtk+ apps

- image selection conversion are now bi-directional. Formats such as
"image/jpg", "image/png", etc. will be delayed-rendered to DIB by using
gdk-pixbuf, i.e. paint.exe can now paste what gimp.exe copied.

- add support of writing bmp to gdk-pixbuf

- fix a bug of the .ico writer- Add support of delayed rendering on Win32

- gtk_clipboard_request_targets() returns all available formats including those
registered by gtk+ apps

- image selection conversion are now bi-directional. Formats such as
"image/jpg", "image/png", etc. will be delayed-rendered to DIB by using
gdk-pixbuf, i.e. paint.exe can now paste what gimp.exe copied.

- add support of writing bmp to gdk-pixbuf

- fix a bug of the .ico writer- Add support of delayed rendering on Win32

- gtk_clipboard_request_targets() returns all available formats including those
registered by gtk+ apps

- image selection conversion are now bi-directional. Formats such as
"image/jpg", "image/png", etc. will be delayed-rendered to DIB by using
gdk-pixbuf, i.e. paint.exe can now paste what gimp.exe copied.

- add support of writing bmp to gdk-pixbuf

- fix a bug of the .ico writer

Should probably get the .ico fix in 2.6.3, the rest looks like a good candidate
for 2.8

2005-02-23  Matthias Clasen  <mclasen@redhat.com>

	* io-ico.c: Fix a typo.  (#168173, Ivan Wong)

I have committed BMP saving to HEAD. 
I'm leaving the delayed rendering stuff to the win32 people.

2005-02-23  Matthias Clasen  <mclasen@redhat.com>

	* gdk-pixbuf-io.c: Document BMP saving support.

	* io-bmp.c: Support saving as BMP. (168173, Ivan Wong)

This won't work outside intel chips: alignment isn't right.  And why are both
signed and unsigned versions needed?

To get it to work, assign the byte-switched value to a variable, then memcpy
from that variable into the buffer.

+#define put16u(buf,data)	{ *(guint16*)(buf) = GUINT16_TO_LE (data); buf += 2; }
+#define put32u(buf,data)	{ *(guint32*)(buf) = GUINT32_TO_LE (data); buf += 4; }
+#define put16(buf,data)		{ *(gint16*)(buf) = GINT16_TO_LE (data); buf += 2; }
+#define put32(buf,data)		{ *(gint32*)(buf) = GINT32_TO_LE (data); buf += 4; }

Matthias: please reconsider this for 2.6.  It's pretty important for getting
something as basic as cut-and-paste working for win32.  The intersection
with non-win32 code isn't _that_ big.

Yes, the Win32 parts will most probably be applied "soon". (Say, in a few
weeks.) Do we have any estimated time for 2.6.3 yet?

> This won't work outside intel chips: alignment isn't right.
Hm.........what's the problem with it? I tested it with a 68K compiler and 
simulator and the whole header seems to be writen correctly.

> And why are both signed and unsigned versions needed?
Physical they are the same. I consider this as a kind of documentation so that 
I know which fields are signed/unsigned without refering to other places. They 
won't increase the actual size of binary but just 2-line of codes.

>what's the problem with it?

It'll segfault on, say, a sparc chip.  It is possible to have fault handlers
that cover up the problem by emulation, though.

I don't really mind having two macros, but I note that you use signed integer
arguments for put32u, for example: 40.  (And 40u looks crazy.)

troll:~> gcc -O2 foo.c
troll:~> ./a.out 
Segmentation Fault (core dumped)
troll:~> uname -a
SunOS troll 5.8 Generic_117350-11 sun4u sparc
troll:~> cat foo.c
typedef unsigned int guint32;
// This is wrong for space, but that's not important here.
#define GUINT32_TO_LE(_x) (_x)
#define put32u(buf,data)        { *(guint32*)(buf) = GUINT32_TO_LE (data); buf
+= 4; }
int main ()
{
  unsigned char BFH_BIH[40], *dst;
  *dst++ = 'B';
  *dst++ = 'M';
  put32u (dst, 42);
  return 0;
}

Bah.

And with dst = BFH_BIH; added, it still barf, though differently:

troll:~> ./a.out 
Bus Error (core dumped)

Created attachment 37936 [details] [review]
fixs the bus error

I see. It seems that I forgot what I learned from the uP course, sigh.

Could BMP saving be included in 2.6? This would be useful for cut/paste and
drag/drop of images from gtk applications to OpenOffice.org. OOo can paste BMP,
but none of the formats which gdk-pixbuf 2.6 can save to.

Fixed the bus error, and backported to 2.6

I am about to commit the Win32 parts to 2.6 and HEAD. 

Is it really so that when you copy an image in GIMP and then paste it into e.g.
MS Paint, the image is first converted to for instance PNG or JPEG format (is it
the image's format on disk that determines this? What if it is a new image?),
and then to BMP and put on the Clipboard? Sounds a bit inefficient. But I guess
there is no easy way to tell GIMP that it should provide it itself as BMP, either?

BTW, how does this relate to the clipboard viewer etc code in gdkdisplay-win32.c?

The code in gdkdisplay-win32.c was added to try what was possible to emulate
for the new Gtk/Clipbaord api, see:
http://mail.gnome.org/archives/gtk-devel-list/2004-May/msg00074.html

From my first once over the patch it isn't used at all. My impression
from the patch here is that I definitely don't like the addition of
further global variables. Also it seems a little bit short in what
should be posible with the clipboard, i.e. if only two apps agree in
what a format to exchange that format should be use. Falling back to
conversions should only be used as last resort.
But with gdk-pixbuf being able to save bitmaps it should be possible
to let the gimp advertize (and later produce) the native "cliboard 
image format". IIRC the gimp registers a list of bitmap formats for
data exchange based on gdk_pixbuf_get_formats() and
gdk_pixbuf_format_is_writable(). See: gimp/app/widgets/gimpcliboard.c

> I definitely don't like the addition of further global variables.

(when used to pass information and state between different parts of the code,
you probaby mean.) True. It would be better to call directly whatever function
the gdk_event_func() will cause to be called from the WM_RENDERFORMAT handler. 

> Falling back to conversions should only be used as last resort.

Well, some conversion is obviously needed from GIMP's internal tiled image data
to something else, but to first convert to PNG and then to BMP is quite
suboptimal. Hmm, it is trivial to fix gimp_clipboard_format_compare() to move
the bmp format to the front of the list (if GDK_WINDOWING_WIN32). Hmm, is it
then gdk_property_change() that should notice if the data is already in
image/bmp format, and put it on the clipboard directly? Or is it
_gdk_win32_selection_convert_to_dib() that should shortcircuit part of its code?
Oh well, some hacking is called for, this can't be hard.

(Reopening as the Win32 part is not done yet.)

> I definitely don't like the addition of further global variables.
I was told to use global variables in a previous bug (bug 163702, #2)

> Also it seems a little bit short in what
> should be posible with the clipboard, i.e. if only two apps agree in
> what a format to exchange that format should be use. Falling back to
> conversions should only be used as last resort.
For example, when I have two instances of gimp, I don't see there's any 
unneccessary converion when I copy and paste between them. They will 
use "image/png" directly (gtk_clipboard_request_targets () does 
return "image/png" and "image/jpeg", no wired "CF_DIB"), CF_DIB is ADDED so 
that *native* win32 apps know there's available image in the clipboard. I don't 
think a gtk app will use CF_DIB, except that the clipboard owner is a native 
win32 app and the gtk app want to retrieve the CF_DIB copied from the native 
app.

> Hmm, is it then gdk_property_change() that should notice if the
> data is already in image/bmp format, and put it on the clipboard
> directly? Or is it _gdk_win32_selection_convert_to_dib() that
> should shortcircuit part of its code?
Note that we do declare the availability 
of "image/png", "image/jpeg", "image/bmp", etc. to the world. If a native win32 
app knows "image/png" and calls:

GetClipboardData (RegisterClipboardFormat ("image/png"));

the clipboard content provider will "put it on the clipboard directly" and 
there's no conversion at all. Conversion only occurs when an native app calls 
GetClipboardData (CF_DIB); More precisely, say, when I copy in an instance of 
gimp, three formats are declared available:

1. custom format "image/png" (directly from gimp)
2. custom format "image/jpeg" (directly from gimp)
3. standard format CF_DIB (we will convert the first from the above two if any 
app requests us to render CF_DIB)

When another gtk app calls gtk_clipboard_request_targets(), it will 
receive "image/png", "image/jpeg" and "image/bmp" (the win32 implementation of 
gdk_selection_convert() will add the availbility of "image/bmp" if it finds 
CF_DIB), redundant conversion will only occur if we request "image/bmp" 
("image/png" -> CF_DIB). The synthesized CF_DIB is needed as many win32 apps 
only know the standard types CF_DIB, CF_BITMAP, etc.

However, I agree that _gdk_win32_selection_convert_to_dib() should handle the 
case where a selection content provider does actually provide the 
format "image/bmp" - it should not use gdk_pixbuf_loader and copy data (except 
BITMAPFILEHEADER) directly.

> I was told to use global variables in a previous bug (bug 163702, #2)

Hmm, yes, although there the situation was quite a bit different. I don't find
the use of the global _delayed_rendering_data in your patch entirely bad as long
as it is thoroughly tested and verified that it works as intended.

> when I copy in an instance of gimp, three formats are declared available:

> 1. custom format "image/png" (directly from gimp)
> 2. custom format "image/jpeg" (directly from gimp)
> 3. standard format CF_DIB

But won't GIMP also advertise image/bmp if running with the gdk-pixbuf where bmp
saving is implemented?

> But won't GIMP also advertise image/bmp if running with
> the gdk-pixbuf where bmp saving is implemented?
No, it doesn't. What I received are "image/png", "image/x-icon" 
and "image/jpeg". I am using gimp, 2.2.1

Are you running the Gimp against gtk HEAD ? Mine gives :

INIT: gimp_initialize
INIT: gimp_real_initialize
INIT: gui_initialize_after_callback
INIT: gimp_restore
INIT: gui_restore_callback
GimpClipboard: writable pixbuf format: image/png
GimpClipboard: writable pixbuf format: image/bmp
GimpClipboard: writable pixbuf format: image/x-bmp
GimpClipboard: writable pixbuf format: image/x-MS-bmp
GimpClipboard: writable pixbuf format: image/x-icon
GimpClipboard: writable pixbuf format: image/jpeg

I am using gtk-2-6. And I just double-checked everything. Of course I have 
applied this bmp-saving patch to gdk-pixbuf since the time I wrote the patch. 
Is it related to the version of my copy of gimp?

However, even if gimp advertises image/bmp, I don't think gdk itself could find 
a trivial mean to distinguish whether the current CIF_DIB in the clipboard is 
copied from a native win32 app or is advertised/synthesized by another instance 
of gdk whose client added target "image/xxx" where xxx is not *bmp and xxx is 
readable by gdk-pixbuf. Applications could avoid redundant conversion by 
placing “image/bmp”at the bottom of the list. Another solution is to drop the 
advertisement of CIF_DIB – simply not serve native win32 applications.

> Applications could avoid redundant conversion by 
> placing “image/bmp”at the bottom of the list.
After thinking a minute, I think the best solution is, as Tor already said, 
skip conversion inside _gdk_win32_selection_convert_to_dib() if "image/bmp" is 
actually advertised by the clipboard owner. Conversion will only occur when:

1. the current clipboard owner only provides formats other than "image/bmp"
2. the current clipboard owner is a native win32 application

I will revise the patch.

> I am using gtk-2-6. And I just double-checked everything. 

Did you try running GIMP with --verbose > gimp.log 2>&1 ? You should see the lines:

GimpClipboard: writable pixbuf format: image/png
GimpClipboard: writable pixbuf format: image/bmp
GimpClipboard: writable pixbuf format: image/x-bmp
GimpClipboard: writable pixbuf format: image/x-MS-bmp
GimpClipboard: writable pixbuf format: image/x-icon
GimpClipboard: writable pixbuf format: image/jpeg

This is with GIMP 2.2, built against GTK+ 2.4, but running against GTK+ 2.6
(from CVS). 

Yup, it was relatively trivial to make _gdk_win32_selection_convert_to_dib()
notice if the data already is image/bmp and in that case just copy the data from
one HGLOBAL to another. But doing this made me think, why the heck do we have to
copy the data in the first place? How many times is the data copied?

Isn't it so that any application on Win32 that stores images to the Windows
Clipboard will provide and accept the CF_DIB format? Is there really any need to
store image/bmp, image/png etc in the Windows Clipboard? At least when
copy/pasteing DIB data, there seems to be way too much copying going on,
prependig BMP file headers to data in CF_DIB format and copying, then stripping
same headers out again and copying, etc.

Created attachment 39552 [details] [review]
revised

> #28
Now that it's very clear. Conversion/Repeated-copying only occurs when:

Conversion
1. clipboard owner is a gtk+ application and it provides formats "image/xxx"
where xxx != *bmp and the requestor is either:
 - a native win32 application who only knows CF_DIB, or
 - another gtk+ application who doesn't request "image/xxx" but "image/*bmp"

Repeated-copying:
1. clipboard owner is a gtk+ application and it provides formats "image/*bmp"
and the requestor is a native win32 application who demands CF_DIB

All other situations, i.e. both the clipboard owner and requestor are gtk+
application and the owner provides formats "image/xxx" where:

- xxx includes *bmp, or
- xxx != *bmp, and the requestor demands "image/yyy" where yyy is the subset of
xxx

requires no conversion/repeated-copying at all.

> Is there really any need to store image/bmp,
> image/png etc in the Windows Clipboard?
IMHO, yes. If my gtk+ application only provides/accepts "image/png", the
clipboard owner/requestor needs to advertise/be aware of the availability of
"image/png" so that my application copies/pastes. Copy and paste between two
gtk+ applications has to be done through the Windows Clipboard.

OK. I'm going to apply you patch to gtk-2-6 now.

(Weekend intervened...) Now committed to HEAD and gtk-2-6. Resolving again.

Ivan, there is a problem with your patch as applied in current HEAD (2.8) and
2.6: gtk_selection_add_targets() might well be called before the GtkWidget has
been realized, i.e. when widget->window is still NULL. At least gtkhtml seems to
do this in gtk_html_init(). 

If gdk_win32_selection_add_targets() is called with a NULL owner it calls
OpenClipboard() with an uninitialized (usually NULL) hwnd, which fails, it
prints a warning message and the functoin returns. It probably should do a
g_return_if_fail (owner!=NULL)?

I don't want it to return silently, as I assume it is a programmer error if
gdk_win32_selection_add_targets() is called with a NULL owner. I.e., is
gtk_selection_add_targets() the right place to call
gdk_win32_selection_add_targets() if the widget isn't realized? Should it be
done also later when the widget is realized? Should gtk_widget_realize() fetch
the current target list, and if nonempty do the gdk_atom_intern() and
gdk_win32_selection_add_targets() loop on it?

Created attachment 50881 [details] [review]
Additional patch

This patch fixes the problem. I call gdk_win32_selection_add_targets() from
gtkselection.c only if the widget is realized. I added code to
gtk_widget_realize() to call gdk_win32_selection_add_targets().

I made the gtk_selection_target_list() function non-static, into an internal
gtk function (declaring it in gtkselection.h and prefixing with an underscore).
Owen, is this OK?

Reopening bug.

gtk_selection_add_targets() says "when I become the owner of this clipboard
in the future, then these are the targets I can support". However, 
gdk_win32_selection_add_targets() actually opens the clipboard and modifies
it. I believe what is in CVS now is only working because GtkClipboard 
does gtk_selection_owner_set_for_display() and gtk_select_add_targets() at
the same time, but this is not a requirement of the GtkSelection API.

You probably want to:

 - If gtk_selection_add_targets() is called when the widget 
   is not the owner of the selection, the target list is stored
   (as it was before the first patch) and nothing else is done.

 - When gtk_selection_owner_set_for_display() called, if there is
   a current target list, call gdk_selection_set_targets() with
   it.

 - If gtk_selection_add_targets() is called when the widget is
   the owner of the selection, the target list is stored and   
   gdk_selection_set_targets() is called immediately.

I'd actually like to see gdk_win32_selection_add_targets() not be 
#ifdef win32 stuff, but rather cross-platform, at least on HEAD.
(see references to gdk_selection_set_targets() ... I think a 
set() API will be better defined than an add() API)

For another clipboard-related problem, see bug #316552

Another problem with the code as it is now after that delayed rendering patch
was applied is that there seems to be confusion in the code between when to use
delayed rendering and when not. After doing a Control-C in the text view demo in
gtk-demo, running a trivial program that lists the clipboard formats available
shows that for instance "TEXT" is available. (It also lists COMPOUND_TEXT,
although I thought that we didn't want to pretend having anything to do with
that crap at all on Win32?)

However, trying to get the clipboard data in the TEXT format invokes the delayed
rendering code through WM_RENDERFORMAT, although delayed rendering apparently
was intended to be used only for images? Getting the data in CF_TEXT format does
work, though, and does not go through the delayed rendering code path. I am more
than a little confused here...

Then there is the clipboard viewer stuff in gdkdisplay-win32.c. This doesn't
seem to actually do anything except print out debugging info (check
_win32_on_clipboard_change()). It is leftover experimetal code (says so in the
comment, more or less) by Hans that, a start of something never actually got
finished. This code has comments like "as we don't do delayed rendering" etc, so
it is out of synch with what actually is done. But then delayed rendering (for
images, at least?) apparently works fine anyway, without a clipboard viewer that
actually would do anything. Sigh.

Would it be best to start using delayed rendering for *all* formats, not just
images?

Created attachment 52409 [details]
Trivial test program to list and dump clipboard data formats.

eh, s/we didn't want to pretend having anything/we wanted to pretend having
nothing/ 

Isn't it a bit odd to call RegisterClipboardFormat in
gdk_win32_selection_add_targets() all the time, also for formats that might
already have been registered? Although that is probably not an expensive call.

> Isn't it a bit odd to call RegisterClipboardFormat in
> gdk_win32_selection_add_targets() all the time, also for formats that might
> already have been registered? Although that is probably not an expensive call.
It's possible to keep our own hash table for it, but user32.dll should have one
already.

> However, trying to get the clipboard data in the TEXT format invokes
> the delayed rendering code through WM_RENDERFORMAT
Expected. "TEXT" is not CF_TEXT, it's one of those text atoms defined in 
gtkselection.c
I can't think of a native win32 or gtk+/win32 application that queries 
for "TEXT". In most cases they query CF_UNICODETEXT/CF_TEXT for text. The only 
way to query "TEXT" is to do a request_contents() with target == gdk_atom_intern
("TEXT").

> although delayed rendering apparently was intended to be used only for images?
Not really. Delayed rendering is carried out for ALL (rich) formats. e.g. Biff, 
and those html/xml table formats for Gnumeric/Excel, which can be text-based or 
binary. The only exception now is _utf8_string which gdk_selection_convert() 
returns when format CF_UNICODETEXT/_cf_utf8_string/CF_TEXT is available. In 
short: pasting text between native/gtk+ or gtk+/gtk+ will not result in delayed 
rendering.

From: http://msdn.microsoft.com/library/default.asp?url=/library/en-
us/winui/winui/windowsuserinterface/dataexchange/clipboard/clipboardoperations.a
sp

"When placing a clipboard format on the clipboard, a window can delay rendering 
the data in that format until the data is needed... This is useful if the 
application supports several clipboard formats, some or all of which are time-
consuming to render"

Actually, AFAIK most win32 application will not delay render text even though 
they do delay render some non-trival formats. If you do a spy++ on Word you 
will find that only text is rendered immediately on copy.

> Getting the data in CF_TEXT format does
> work, though, and does not go through the delayed rendering code path.
Format will be delayed rendered iff it's registered with SetClipboardData
(formatid, NULL). _utf8_string is the only format that's now registered with 
data != NULL, i.e. not delayed renered. See gdk_win32_selection_add_targets() 
and gdk_selection_convert().

Yes, no normal Win32 application will ask for the formats TEXT or STRING (and
GTK+ apps hopefully prefer UTF8_STRING), but why then advertise them at all
(especially as it won't work anyway, see below)?

Maybe it would be best to just ifdef out (when GDK_WINDOWING_WIN32) from gtk/*.c
all the uses of STRING, TEXT, and especially COMPOUND_TEXT which we definitely
don't want to have anything to do with at all in the win32 backend. Or maybe
cleaner to just prune out attempts to use those in the gdk/win32 code.

> _utf8_string is the only format that's now registered with 
> data != NULL, i.e. not delayed renered. 

Hmm, but if I call GetClipboardData() to get TEXT or STRING format (even though
no "normal" Win32 or GTK+ app would do it, if the format is advertised, one
should expect it to work), it goes through the non-delayed-rendering code path
and then errors at gdkproperty-win32.c:290 as OpenClipboard() shouldn't be
called when handling WM_RENDERFORMAT. Try it! Run gtk-demo, copy some text to
the clipboard, then run the attached test program "clipboard -l" to get the
format number for STRING, then "clipboard -g 49624" (or whatever number STRING
or TEXT has for you).

> it goes through the non-delayed-rendering code path
> and then errors at gdkproperty-win32.c:290 as OpenClipboard() shouldn't be
> called when handling WM_RENDERFORMAT
I can't reproduce it. Both STRING and TEXT are delayed rendered and "clipboard -
g" shows the correct text.

I tested with a clean checkout of gtk-2-6.

Eh, odd. I don't understand, I get the error both with 2.6 and 2.8... 

Just looked closer, in gdk_property_change():

if ((type == GDK_TARGET_STRING && GetACP () == 1252) ||
	  type == _utf8_string)

that answers why I had no problem: my ACP == 950
Any idea why the GetACP() check is needed? Anyway the type of "TEXT" is 
GDK_TARGET_STRING, the better solution is, as Tor suggested before, to filter 
all these GDK_TARGET_STRING foramts in gdk_win32_selection_add_targets().

Ah yes, that explains it. The reason for the GetACP()==1252 test is that the
encoding for STRING is in ISO-8859-1, which most closely resembles CP1252.

I'll check how easy it is to filter out all attempts to advertise TEXT, STRING
or COMPOUND_TEXT. One perhaps only theoretical risk is that some application
calls GDK directly wants to advertise only STRING, and thus then would end up
advertising no format at all. Hmm. Probably that just doesn't happen.

Created attachment 52438 [details] [review]
proposed patch for filtering string atoms

Is this ok?
Two string comparasions are really not ideal, but text_atom and ctext_atom are
gtk+ stuff and it's not trival to know what will be assigned to
selection_data->type in gtk_selection_data_set() from gdk.

Hmm. Shouldn't STRING, TEXT and COMPOUND_TEXT be filtered out also in
gdk_property_change()?

Anyway, this talk about TEXT, STRING and COMPOUND_TEXT was just a digression.

Ivan, while I have your attention ;-), what do you think about the actual issue
that caused me to reopen this bug (Comment #33), and Owen's Comment #36 ?

> Hmm. Shouldn't STRING, TEXT and COMPOUND_TEXT be filtered out also in
> gdk_property_change()?
Is gdk_property_change() only for clipboard property?

> Ivan, while I have your attention ;-), what do you think about the actual issue
> that caused me to reopen this bug (Comment #33), and Owen's Comment #36 ?
I think what Owen said in Comment #36 is sane, and is not hard to implement?
Anyone want to go for it? I can try if nobody is free.

BTW, if you insert a debugging printout before the
SetClipboardData(formatid,NULL) call in gdk_win32_selection_add_targets() you
will notice that when using Control-C in a text entry widget, the code will call
SetClipboardData(formatid,NULL) (i.e., claim to provide delayed rendering) also
for the formats "text/plain", "text/plain;charset=CP<n>" where <n> is you system
codepage, and "text/plain;charset=utf-8".

Should these be filtered out also? Or will this be a never-ending chase, what
then if some applications claims to provide a format "application/whatever"?

Should the code be changed to call SetClipboardData(formatid, NULL) only after
it has recognized that the format really is an image format, as image formats
are the only ones the code actually is prepared to use delayed rendering on? 

Oh, this is a mess...

Well, at least I am pretty certain now that the clipboard viewer code in
gdkdisplay-win32.c (which after all is admitted in the comment to be just
"testcode") doesn't do anything useful. In fact the debugging output for
WM_DRAWCLIPBOARD seems to cause a problem with Firefox, for some odd reason: If
gtk-demo is running with GDK_DEBUG=dnd, copying from Firefox to the clipboard
fails. (This probably is a bug in Firefox, though, as copying from other apps to
the clipboard in the same situation works fine.) Also, shouldn't the
WM_CHANGECBCHAIN handler pass the message on to _hwnd_next_viewer in case wparam
isn't _hwnd_next_viewer? 

I'll just remove the clipboard viewer code, and change
gdk_display_supports_selection_notification() and
gdk_display_request_selection_notification() to always just return FALSE. This
will at least clear up one confusing part of the code.

Removed clipboard viewer code from HEAD and gtk-2-8:

2005-10-14  Tor Lillqvist  <tml@novell.com>

	* gdk/win32/gdkdisplay-win32.c: Remove the clipboard viewer code.
	It didn't really do anything useful, and was just confusing and
	incomplete. Comments claimed we don't do delayed rendering, but in
	fact we do, for images. (The delayed rendering code has other
	problems, though, see #168173.) The clipboard viewer code was
	probably even buggy (the WM_CHANGECBCHAIN handled didn't propagate
	the message when necessary). It was just test code, it said so in
	a comment. Add something similar back later if necessary.

	(_win32_on_clipboard_change,
	_gdk_win32_register_clipboard_notification): Remove.

	(gdk_display_supports_selection_notification,
	gdk_display_request_selection_notification): Always just return
	FALSE. We didn't generate any GDK_OWNER_CHANGE events anywhere.

Thinking more about this, and considering Owen't Comment #36, I am getting more
convinced that using delayed rendering for all formats would be the right thing
to do. Wouldn't it correspond much better to how X11 selections work?

For instance, isn't the GDK_SELECTION_REQUEST we send to ourselves at the end of
gdk_selection_owner_set_for_display() completely bogus for other types of data
than text? Note that the GDK_SELECTION_REQUEST sent has a hardcoded UTF8_STRING
as target! Such crap... (can I say as I wrote it myself ;-) We cannot know when
gdk_selection_owner_set_for_display() is called what kind of data will
eventually be stored in the selection property. Sending ourselves a
GDK_SELECTION_REQUEST for UTF8_STRING at that point can't be right in general,
although it perhaps happens to have no ill effects apparently in GIMP when the
selection is an image, and works when the selection really is text.

Committed an intermediate change that at least removes the formats
'COMPOUND_TEXT', 'TEXT', 'STRING' and 'SAVE_TARGETS' from being registered and
advertised.

2005-11-01  Tor Lillqvist  <tml@novell.com>

	* gdk/win32/gdkprivate-win32.h
	* gdk/win32/gdkglobals-win32.c
	* gdk/win32/gdkmain-win32.c (_gdk_windowing_init): Add more
	pre-interned GdkAtoms and pre-registered clipboard formats. Sort
	their declarations, definitions and assignments into a more
	logical and consistent order.

	* gdk/win32/gdkmain-win32.c (_gdk_win32_cf_to_string): Include the
	CF_ prefix for the predefined clipboard format names. Put quotes
	around registered format names to distinguish them.
	
	* gdk/win32/gdkproperty-win32.c (gdk_property_change): Return
	immediately with a warning if the property type is STRING, TEXT,
	COMPOUND_TEXT or SAVE_TARGETS, as these are X11-specific that we
	should never pretend to handle on Win32. Handle only UTF8_STRING
	here, other formats with delayed rendering. Use \uc1 instead of
	\uc0 when generating Rich Text Format for easier testability on
	XP, where WordPad misinterprets \uc0 encoded characters. Add more
	GDK_NOTE debugging output for Clipboard operations.

	* gdk/win32/gdkselection-win32.c: Debugging printout improvements.
	(gdk_selection_convert): Don't pretent to handle STRING, just
	UTF8_STRING. Streamline error handling, don't unnecessarily have a
	GError which then isn't used for anything anyway if it gets set.
	(gdk_win32_selection_add_targets): Skip also STRING, TEXT,
	COMPOUND_TEXT and SAVE_TARGETS in addition to UTF8_STRING.

so, can this be bug be closed now? tor?
this is NEEDINFO for nearly a year now.

Please wait for an ack from Ivan.

I don't actually see why it is in NEEDINFO right now.  Can I NEEDINFO
the NEEDINFO?  :-)

Tested with Gnumeric and it works as expected for both text and graphics.