Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clipboard: fix a bug when pasting image to LibreOffice #3120

Merged

Conversation

metalefty
Copy link
Member

While here, embed correct file size in BMP file header.

Fixes: #3102
Sponsored by: Krämer Pferdesport GmbH & Co KG

@metalefty metalefty force-pushed the v0.10-libreoffice-image-clipboard branch from 8f8b4db to 2290a1b Compare June 17, 2024 11:45
@metalefty metalefty changed the title clipboard: fix a bug pasting image to LibreOffice clipboard: fix a bug when pasting image to LibreOffice Jun 17, 2024
While here, embed correct file size in BMP file header.

Fixes:          neutrinolabs#3102
Sponsored by:   Krämer Pferdesport GmbH & Co KG
@metalefty metalefty force-pushed the v0.10-libreoffice-image-clipboard branch from 2290a1b to 4968a34 Compare June 17, 2024 12:08
@metalefty
Copy link
Member Author

Rebased onto #3088 to fix FreeBSD CI.

for (int i = 5, j = 0; i >= 2; i--, j += 8)
#endif
{
bmp_file_header[i] = ((bmp_size >> j) & 0xff);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@matt335672 Can you have a look at this area especially? I'm not sure if this is a good way to embed file size in BMP file header.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that could do with tidying up.

Personally, I'd use the stream functions we already have. They've been well-tested, an xrdp developer should be familiar with them, and a lot of magic numbers can be avoided. Something like this maybe (compiles, but not tested):-

    /* Assemble bmp header */
    /* See https://en.wikipedia.org/wiki/BMP_file_format#Bitmap_file_header */
    make_stream(hdr_s);
    if (hdr_s == 0)
    {
        g_free(g_clip_c2s.data);
        g_clip_c2s.total_bytes = 0;
        return 0;
    }
    init_stream(hdr_s, BMPFILEHEADER_LEN);
    out_uint8(hdr_s, 'B');
    out_uint8(hdr_s, 'M');
    out_uint32_le(hdr_s, g_clip_c2s.total_bytes);
    out_uint16_le(hdr_s, 0);
    out_uint16_le(hdr_s, 0);
    out_uint32_le(hdr_s, BMPFILEHEADER_LEN + 40);

    /* Copy header and data to output stream */
    g_memcpy(g_clip_c2s.data, hdr_s->data, BMPFILEHEADER_LEN);
    in_uint8a(s, g_clip_c2s.data + BMPFILEHEADER_LEN, len);
    free_stream(hdr_s);

You can remove g_bmp_image_header, and simply define BMPFILEHEADER_LEN as 14.

It's not as efficient, but that probably doesn't matter. It's not being called that often to need to optimise it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it worked and applied tidy-up. I also replaced the magic number 40 with BMPINFOHEADER_LEN.

Sponsored by:   Krämer Pferdesport GmbH & Co KG
Copy link
Member

@matt335672 matt335672 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Also needs to go into devel

@metalefty metalefty merged commit 0aa3a67 into neutrinolabs:v0.10 Jun 18, 2024
13 checks passed
@metalefty metalefty deleted the v0.10-libreoffice-image-clipboard branch June 18, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants