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

Different results on Linux and Windows when GraphicsUnit.Point + transformation matrix are used #545

Closed
shibaev opened this issue May 7, 2019 · 2 comments · Fixed by #551

Comments

@shibaev
Copy link
Contributor

shibaev commented May 7, 2019

The following C# code produces different results on Linux and Windows - the black rectangle is located at different places:

const float TargetResolution = 300;
double scaleFactor = TargetResolution / 72;
using (Bitmap bitmap = new Bitmap((int)(612 * scaleFactor), (int)(792 * scaleFactor)))
{
    bitmap.SetResolution(TargetResolution, TargetResolution);

    using (System.Drawing.Graphics gr = System.Drawing.Graphics.FromImage(bitmap))
    {
        gr.PageUnit = GraphicsUnit.Point;

        gr.Transform = new Matrix(1, 0, 0, 1, 360, 111);
        gr.FillRectangle(Brushes.Black, 0, 0, 74, 72);
    }

    bitmap.Save("result.png");
}

Linux output:
result-linux
I tried both libgdiplus 4.2-2 and the latest version compiled with master (dba6fc6) - the result is the same.

Windows output:
result-win

@shibaev
Copy link
Contributor Author

shibaev commented May 28, 2019

Here is the test for libgdiplus to reproduce the issue:

static void test_issue545()
{
    GpStatus status;
    GpBitmap* bitmap;
    GpGraphics* graphics;
    GpSolidFill* brush;
    CLSID png_clsid = { 0x557cf406, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } };
    const float TargetResolution = 300;
    double scaleFactor = TargetResolution / 72;
    
    status = GdipCreateBitmapFromScan0((int)(612 * scaleFactor), (int)(792 * scaleFactor), 0, PixelFormat32bppARGB, NULL, &bitmap);
    assertEqualInt(status, Ok);

    status = GdipBitmapSetResolution(bitmap, TargetResolution, TargetResolution);
    assertEqualInt(status, Ok);

    status = GdipGetImageGraphicsContext(bitmap, &graphics);
    assertEqualInt(status, Ok);

    status = GdipSetPageUnit(graphics, UnitPoint);
    assertEqualInt(status, Ok);

    const ARGB FillColor = 0xFF000000;
    status = GdipCreateSolidFill(FillColor, &brush);
    assertEqualInt(status, Ok);

    int rectX = 360;
    int rectY = 111;
    int rectWidth = 74;
    int rectHeight = 72;
#ifndef PASS
    status = GdipTranslateWorldTransform(graphics, rectX, rectY, MatrixOrderPrepend);
    assertEqualInt(status, Ok);

    status = GdipFillRectangleI(graphics, brush, 0, 0, rectWidth, rectHeight);
    assertEqualInt(status, Ok);
#else
    status = GdipFillRectangleI(graphics, brush, rectX, rectY, rectWidth, rectHeight);
    assertEqualInt(status, Ok);
#endif

    int points[] = {
        rectX,
        rectY,
        rectX + rectWidth / 2,
        rectY + rectHeight / 2,
        rectX + rectWidth - 1,
        rectY + rectHeight - 1
    };
    for (int i = 0; i < sizeof(points) / sizeof(points[0]); i += 2)
    {
        ARGB color;
        status = GdipBitmapGetPixel(bitmap, points[i], points[i + 1], &color);
        assertEqualInt(status, Ok);
        assertEqualInt(color, 0);

        status = GdipBitmapGetPixel(bitmap, (int)(points[i] * scaleFactor) + 1, (int)(points[i + 1] * scaleFactor) + 1, &color);
        assertEqualInt(status, Ok);
        assertEqualInt(color, FillColor);
    }

    // Uncomment to visually compare outputs
    // WCHAR* filePath = createWchar("test_issue545.png");
    // status = GdipSaveImageToFile(bitmap, filePath, &png_clsid, NULL);

    GdipDisposeImage((GpImage*)bitmap);
    GdipDeleteGraphics(graphics);
    GdipDeleteBrush(brush);
}

The test will pass if you define PASS symbol or inverse "#ifndef PASS" condition.

@shibaev
Copy link
Contributor Author

shibaev commented May 28, 2019

The root cause - GpGraphics::copy_of_ctm is passed to Cairo as is. Instead it should respect page_unit for translation part (x0 and y0).

shibaev added a commit to BitMiracle/libgdiplus that referenced this issue May 28, 2019
* test_world_transform_respects_page_unit_document - tests
GdipSetWorldTransform and UnitDocument
* test_world_transform_respects_page_unit_point - tests
GdipTranslateWorldTransform and UnitPoint
shibaev added a commit to BitMiracle/libgdiplus that referenced this issue May 28, 2019
)

* general.c - implement gdip_cairo_set_matrix function that converts
translation part of a matrix from page units to cairo coordinates
* graphics-cairo.c - fix typo when input parameter 'matrix' was not used
* customlinecap.c, graphics-cairo.c, graphics.c, image.c, pen.c - use
gdip_cairo_set_matrix instead of cairo_set_matrix when input matrix is
expressed in page units
akoeplinger pushed a commit that referenced this issue Jul 31, 2019
…iro (#551)

* Add failed tests for page unit and world transform dependency (#545)

* test_world_transform_respects_page_unit_document - tests
GdipSetWorldTransform and UnitDocument
* test_world_transform_respects_page_unit_point - tests
GdipTranslateWorldTransform and UnitPoint

* Respect page units when passing transformation matrix to cairo (#545)

* general.c - implement gdip_cairo_set_matrix function that converts
translation part of a matrix from page units to cairo coordinates
* graphics-cairo.c - fix typo when input parameter 'matrix' was not used
* customlinecap.c, graphics-cairo.c, graphics.c, image.c, pen.c - use
gdip_cairo_set_matrix instead of cairo_set_matrix when input matrix is
expressed in page units

* Fix clipping of non-rectangular paths (#547, #552)

Clipping of paths and path-based regions was broken in
5dd3b5d. This commit fixes the issue.

* graphics-cairo.c - do not pass NULL matrix to GdipGetRegionScansCount
and GdipGetRegionScans

* testclip.c - add test for clipping of paths and regions based on paths

* Fix indent in test

Fixes #545.
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 a pull request may close this issue.

1 participant