Re: [PATCH] macdrv: Don't update clipboard if its content didn't change

[Ken Thomases]

On May 25, 2017, at 1:22 PM, Piotr Caban  wrote:
> Signed-off-by: Piotr Caban
>
> dlls/winemac.drv/clipboard.c       | 34 ++++++++++------------------------
> dlls/winemac.drv/cocoa_clipboard.m | 18 ++++++++++++++++++
> dlls/winemac.drv/macdrv_cocoa.h    |  1 +
> 3 files changed, 29 insertions(+), 24 deletions(-)
>
> +int macdrv_has_pasteboard_changed(void)
> +{
> - __block int new_change_count;
> - int ret;
> -
> - OnMainThread(^{
> -
>    NSPasteboard* pb = [NSPasteboard generalPasteboard];
> -
>    new_change_count = [pb changeCount];
> - });
> -
> - ret = (change_count != new_change_count);
> - change_count = new_change_count;
> - return ret;
>
> +}

Unfortunately, -[NSPasteboard changeCount] isn't incremented for all changes to the pasteboard.  For example, if the owner replaces the data for an existing type, it doesn't change.  I'd have to check again, but I think it doesn't even change if the owner adds a new data type.  I think it only changes when the pasteboard is cleared and rebuilt by -clearContents or -declareTypes:owner:.

-Ken

[PATCH] macdrv: Don't update clipboard if its content didn't change

[Piotr Caban]

> Unfortunately, -[NSPasteboard changeCount] isn't incremented for all changes to the pasteboard.  For example, if the owner replaces the data for an existing type, it doesn't change.  I'd have to check again, but I think it doesn't even change if the owner adds a new data type.  I think it only changes when the pasteboard is cleared and rebuilt by -clearContents or -declareTypes:owner:.

I wonder if applications that are doing it are using pasteboard API correctly. The documentation says that “application can determine whether the current data in the pasteboard is the same as the data it last received” by checking changeCount value. I’ve also checked the code of open source clipboard manager that works on Mac (Flycut) - as far as I can see it's looking for clipboard changes this way.

[PATCH] macdrv: Don't update clipboard if its content didn't change

[Ken Thomases]

On May 25, 2017, at 5:26 PM, Piotr Caban  wrote:
> > Unfortunately, -[NSPasteboard changeCount] isn't incremented for all changes to the pasteboard.  For example, if the owner replaces the data for an existing type, it doesn't change.  I'd have to check again, but I think it doesn't even change if the owner adds a new data type.  I think it only changes when the pasteboard is cleared and rebuilt by -clearContents or -declareTypes:owner:.
>
> I wonder if applications that are doing it are using pasteboard API correctly.

…
> I’ve also checked the code of open source clipboard manager that works on Mac (Flycut) - as far as I can see it's looking for clipboard changes this way.

Well, I suspect that the vast majority of the time, apps work with the pasteboard in a way which does allow another app to poll the change count to be informed of the changes.  I'd even go so as to say that that's the "normal" way of working with the pasteboard.  However, the API allows for the operations that in fact change the data on the pasteboard but which don't increment the change count.  I don't think such uses of the API are incorrect, just uncommon.
> The documentation says that “application can determine whether the current data in the pasteboard is the same as the data it last received” by checking changeCount value.

The docs also say, "The change count … increments every time the contents of the pasteboard changes (a new owner is declared)"[1] and, "The change count … increments each time the pasteboard ownership changes."[2]  Those suggest it's tied to the methods which specifically take ownership of (and clear) the pasteboard.

[1] https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/P...
[2] https://developer.apple.com/reference/appkit/nspasteboard/1533544-changecoun...

I'll have to run some tests again to see exactly which operations do and don't increment the change count.  I'll let you know what I find.

It may be that your patch is better for the vast majority of the cases such that we should commit it.  We'll just accept incorrect behavior for the (hopefully) rare aberrant uses of the API that I'm concerned about.  Or maybe we can take a belt-and-suspenders approach and check both the change count and the list of types.

-Ken

[PATCH] macdrv: Don't update clipboard if its content didn't change

[Piotr Caban]

> I'll have to run some tests again to see exactly which operations do and don't increment the change count.  I'll let you know what I find.
>
> It may be that your patch is better for the vast majority of the cases such that we should commit it.  We'll just accept incorrect behavior for the (hopefully) rare aberrant uses of the API that I'm concerned about.  Or maybe we can take a belt-and-suspenders approach and check both the change count and the list of types.

I’ve done some initial testing:

[myPasteboard clearContents];
clearContent increments the changeCount.

[myPasteboard setString:@"test" forType:NSStringPboardType];
        NSLog(@"changeCount %ld", [myPasteboard changeCount]);
        [NSThread sleepForTimeInterval:2];
        [myPasteboard setString:@"test2" forType:NSStringPboardType];
        NSLog(@“changeCount %ld", [myPasteboard changeCount]);
In this case changeCount doesn’t change no matter if other applications read the pasteboard data between setString calls.

[myPasteboard addTypes:[NSArray arrayWithObject:NSRTFPboardType] owner:nil];
        NSLog(@“addTypes changeCount %ld”, [myPasteboard changeCount]);
        [myPasteboard setString:rtf forType:NSRTFPboardType];
        NSLog(@"new data type changeCount %ld", [myPasteboard changeCount]);
Adding new clipboard format doesn’t change changeCount. Setting value for newly added format also doesn’t change it.

[myPasteboard declareTypes:[NSArray arrayWithObjects:NSStringPboardType, NSURLPboardType, nil] owner:nil];
declareTypes changes changeCount value.

Thank you,
Piotr

[PATCH] macdrv: Don't update clipboard if its content didn't change

[Ken Thomases]

On May 26, 2017, at 1:23 AM, Piotr Caban  wrote:
> > I'll have to run some tests again to see exactly which operations do and don't increment the change count.  I'll let you know what I find.
> >
> > It may be that your patch is better for the vast majority of the cases such that we should commit it.  We'll just accept incorrect behavior for the (hopefully) rare aberrant uses of the API that I'm concerned about.  Or maybe we can take a belt-and-suspenders approach and check both the change count and the list of types.
>
> I’ve done some initial testing:
>
> [myPasteboard clearContents];
> clearContent increments the changeCount.
>
> [myPasteboard setString:@"test" forType:NSStringPboardType];
>        NSLog(@"changeCount %ld", [myPasteboard changeCount]);
>        [NSThread sleepForTimeInterval:2];
>        [myPasteboard setString:@"test2" forType:NSStringPboardType];
>        NSLog(@“changeCount %ld", [myPasteboard changeCount]);
> In this case changeCount doesn’t change no matter if other applications read the pasteboard data between setString calls.
>
> [myPasteboard addTypes:[NSArray arrayWithObject:NSRTFPboardType] owner:nil];
>        NSLog(@“addTypes changeCount %ld”, [myPasteboard changeCount]);
>        [myPasteboard setString:rtf forType:NSRTFPboardType];
>        NSLog(@"new data type changeCount %ld", [myPasteboard changeCount]);
> Adding new clipboard format doesn’t change changeCount. Setting value for newly added format also doesn’t change it.
>
> [myPasteboard declareTypes:[NSArray arrayWithObjects:NSStringPboardType, NSURLPboardType, nil] owner:nil];
> declareTypes changes changeCount value.

Thanks for running those tests.  They seem to confirm what I remembered.

Since the vast majority of apps will call -clearContents or -declareTypes:owner: each time they modify the pasteboard, I'm willing to accept a version of this change.  Do you mind restoring the tracking and checking of last_types, so that we grab the Win32 clipboard if either the change count or the available types has changed?

If this approach causes problems in the future, we'll reconsider at that time.

Thanks,
Ken

[PATCH] macdrv: Don't update clipboard if its content didn't change

[Piotr Caban]

On 05/30/17 06:40, Ken Thomases wrote:
> Since the vast majority of apps will call -clearContents or -declareTypes:owner: each time they modify the pasteboard, I'm willing to accept a version of this change.  Do you mind restoring the tracking and checking of last_types, so that we grab the Win32 clipboard if either the change count or the available types has changed?

Sure, I'll send a new patch.

Thanks,
Piotr