[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-4-0' - common/Png.hpp

[Ashod Nakashian (via logerrit)]

 common/Png.hpp |   61 ++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 20 deletions(-)

New commits:
commit 2ff0409defd9996b0866bc6b10619dc677d312e6
Author:     Ashod Nakashian <ashod.nakashian at collabora.co.uk>
AuthorDate: Sat Sep 21 21:10:18 2019 -0400
Commit:     Jan Holesovsky <kendy at collabora.com>
CommitDate: Thu Oct 10 14:59:09 2019 +0200

    wsd: fix variable might be clobbered by ‘longjmp’ or ‘vfork’
    
    As the comment details, this avoids having C++ objects
    in the same frame as setjmp, which may reset their
    contents without the dtor getting called.
    
    Change-Id: I851ae8bffb4356d465a25dfc815a1fecb489fa30
    Reviewed-on: https://gerrit.libreoffice.org/79338
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>
    Reviewed-on: https://gerrit.libreoffice.org/80319
    Reviewed-by: Jan Holesovsky <kendy at collabora.com>
    Tested-by: Jan Holesovsky <kendy at collabora.com>

diff --git a/common/Png.hpp b/common/Png.hpp
index 0d40a37a0..34328ce8c 100644
--- a/common/Png.hpp
+++ b/common/Png.hpp
@@ -111,13 +111,17 @@ unpremultiply_data (png_structp /*png*/, png_row_infop row_info, png_bytep data)
     }
 }
 
-// Sadly, older libpng headers don't use const for the pixmap pointer parameter to
-// png_write_row(), so can't use const here for pixmap.
-inline
-bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY,
-                          int width, int height,
-                          int bufferWidth, int bufferHeight,
-                          std::vector<char>& output, LibreOfficeKitTileMode mode)
+/// This function uses setjmp which may clobbers non-trivial objects.
+/// So we can't use logging or create complex C++ objects in this frame.
+/// Specifically, logging uses std::string objects, and GCC gives the following:
+/// error: variable ‘__capacity’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
+/// In practice, this is bogus, since __capacity is has a trivial type, but this error
+/// shows up with the sanitizers. But technically we shouldn't mix C++ objects with setjmp.
+/// Sadly, older libpng headers don't use const for the pixmap pointer parameter to
+/// png_write_row(), so can't use const here for pixmap.
+inline bool impl_encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY,
+                                      int width, int height, int bufferWidth, int bufferHeight,
+                                      std::vector<char>& output, LibreOfficeKitTileMode mode)
 {
     if (bufferWidth < width || bufferHeight < height)
     {
@@ -154,8 +158,6 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY,
         png_set_write_user_transform_fn (png_ptr, unpremultiply_data);
     }
 
-    auto a = std::chrono::steady_clock::now();
-
     for (int y = 0; y < height; ++y)
     {
         size_t position = ((startY + y) * bufferWidth * 4) + (startX * 4);
@@ -164,17 +166,6 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY,
 
     png_write_end(png_ptr, info_ptr);
 
-    auto b = std::chrono::steady_clock::now();
-
-    std::chrono::milliseconds::rep duration = std::chrono::duration_cast<std::chrono::milliseconds>(b-a).count();
-
-    static std::chrono::milliseconds::rep totalDuration = 0;
-    static int nCalls = 0;
-
-    totalDuration += duration;
-    nCalls += 1;
-    LOG_TRC("Average PNG compression time after " << std::to_string(nCalls) << " calls: " << (totalDuration / static_cast<double>(nCalls)));
-
     png_destroy_write_struct(&png_ptr, &info_ptr);
 
 #ifdef IOS
@@ -190,6 +181,36 @@ bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY,
     return true;
 }
 
+/// Sadly, older libpng headers don't use const for the pixmap pointer parameter to
+/// png_write_row(), so can't use const here for pixmap.
+inline bool encodeSubBufferToPNG(unsigned char* pixmap, size_t startX, size_t startY, int width,
+                                 int height, int bufferWidth, int bufferHeight,
+                                 std::vector<char>& output, LibreOfficeKitTileMode mode)
+{
+    const auto start = std::chrono::steady_clock::now();
+
+    const bool res = impl_encodeSubBufferToPNG(pixmap, startX, startY, width, height, bufferWidth,
+                                               bufferHeight, output, mode);
+    if (Log::traceEnabled())
+    {
+        const auto end = std::chrono::steady_clock::now();
+
+        std::chrono::milliseconds::rep duration
+            = std::chrono::duration_cast<std::chrono::milliseconds>(end - start).count();
+
+        static std::chrono::milliseconds::rep totalDuration = 0;
+        static int nCalls = 0;
+
+        totalDuration += duration;
+        ++nCalls;
+        LOG_TRC("PNG compression took "
+                << duration << " ms (" << output.size() << " bytes). Average after " << nCalls
+                << " calls: " << (totalDuration / static_cast<double>(nCalls)));
+    }
+
+    return res;
+}
+
 inline
 bool encodeBufferToPNG(unsigned char* pixmap, int width, int height,
                        std::vector<char>& output, LibreOfficeKitTileMode mode)