Merge lp:~vanvugt/mir/cursor-stride into lp:mir

[Daniel van Vugt]

Fix cursor stride calculation that could lead to a corrupt cursor
image (LP: #1391975)

[Kevin DuBois (kdub)]

47 + write_buffer_data_locked(lg, padded, padded_size);
48 + delete[] padded;
If write_buffer_data_locked throws (which it can if gbm_bo_write fails), we leak padded.

looks good otherwise

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

Lack of exception safety. One solution:

25 + auto padded = new uint8_t[padded_size];

    std::unique_ptr<uint8_t[]> const padded{new uint8_t[padded_size]};

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2062
http://jenkins.qa.ubuntu.com/job/mir-ci/2123/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/149
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/149
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/112
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/112/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/129
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/129/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3413
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15897

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2123/rebuild

review: Approve (continuous-integration)

[Robert Carr (robertcarr)]

Ok with already mentioned fix.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2063
http://jenkins.qa.ubuntu.com/job/mir-ci/2129/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/156
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/156
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/118
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/118/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/136
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/136/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/3416
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/15910

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/2129/rebuild

review: Approve (continuous-integration)

[Cemil Azizoglu (cemil-azizoglu)]

Approved after the aforementioned fixes.

review: Approve

[Daniel van Vugt (vanvugt)]

review: Approve

=== modified file 'src/platform/graphics/mesa/cursor.cpp'
--- src/platform/graphics/mesa/cursor.cpp	2014-10-01 06:25:56 +0000
+++ src/platform/graphics/mesa/cursor.cpp	2014-11-18 03:28:01 +0000
@@ -101,28 +101,35 @@
 
 void mgm::Cursor::pad_and_write_image_data_locked(std::lock_guard<std::mutex> const& lg, CursorImage const& image)
 {
-    uint32_t const* image_argb = static_cast<uint32_t const*>(image.as_argb_8888());
+    auto image_argb = static_cast<uint8_t const*>(image.as_argb_8888());
     auto image_width = image.size().width.as_uint32_t();
     auto image_height = image.size().height.as_uint32_t();
+    auto image_stride = image_width * 4;
 
     if (image_width > buffer_width || image_height > buffer_height)
     {
         BOOST_THROW_EXCEPTION(std::logic_error("Image is too big for GBM cursor buffer"));
     }
     
-    uint32_t pixels[buffer_width*buffer_height] {};
-    // 'pixels' is initialized to transparent so we just need to copy the initial image
-    //  in to the top left corner.
-    for (unsigned int i = 0; i < image_height; i++)
+    size_t buffer_stride = gbm_bo_get_stride(buffer);  // in bytes
+    size_t padded_size = buffer_stride * buffer_height;
+    auto padded = std::unique_ptr<uint8_t[]>(new uint8_t[padded_size]);
+    size_t rhs_padding = buffer_stride - image_stride;
+
+    uint8_t* dest = &padded[0];
+    uint8_t const* src = image_argb;
+
+    for (unsigned int y = 0; y < image_height; y++)
     {
-        for (unsigned int j = 0; j < image_width; j++)
-        {
-            pixels[buffer_width*i+j] = image_argb[image_width*i + j];
-        }
+        memcpy(dest, src, image_stride);
+        memset(dest + image_stride, 0, rhs_padding);
+        dest += buffer_stride;
+        src += image_stride;
     }
 
-    auto const count = buffer_width * buffer_height * sizeof(uint32_t);
-    write_buffer_data_locked(lg, pixels, count);
+    memset(dest, 0, buffer_stride * (buffer_height - image_height));
+
+    write_buffer_data_locked(lg, &padded[0], padded_size);
 }
 
 void mgm::Cursor::show(CursorImage const& cursor_image)

=== modified file 'tests/unit-tests/graphics/mesa/test_cursor.cpp'
--- tests/unit-tests/graphics/mesa/test_cursor.cpp	2014-10-01 06:25:56 +0000
+++ tests/unit-tests/graphics/mesa/test_cursor.cpp	2014-11-18 03:28:01 +0000
@@ -300,7 +300,12 @@
     };
 
     // We expect a full 64x64 pixel write as we will pad missing data with transparency.
-    size_t const buffer_size_bytes{64 * 64 * sizeof(uint32_t)};
+    size_t const height = 64;
+    size_t const width = 64;
+    size_t const stride = width * 4;
+    size_t const buffer_size_bytes{height * stride};
+    ON_CALL(mock_gbm, gbm_bo_get_stride(_))
+        .WillByDefault(Return(stride));
     EXPECT_CALL(mock_gbm, gbm_bo_write(mock_gbm.fake_gbm.bo, ContainsASingleWhitePixel(), buffer_size_bytes));
 
     cursor.show(SinglePixelCursorImage());