From b3af2af20adad0091ac5b8701b667b90a7f560f5 Mon Sep 17 00:00:00 2001 From: yale Date: Sun, 31 Dec 2017 01:44:59 -0800 Subject: [PATCH 1/6] Disable GTK3's motion event compression which was making drawing lag. This could be related to https://bugs.launchpad.net/inkscape/+bug/1723247 --- src/display/sp-canvas.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/display/sp-canvas.cpp b/src/display/sp-canvas.cpp index 20ad27bf5d..ffcfe12ba6 100644 --- a/src/display/sp-canvas.cpp +++ b/src/display/sp-canvas.cpp @@ -1083,6 +1083,7 @@ void SPCanvas::handle_realize(GtkWidget *widget) GdkWindow *window = gdk_window_new (gtk_widget_get_parent_window (widget), &attributes, attributes_mask); gtk_widget_set_window (widget, window); gdk_window_set_user_data (window, widget); + gdk_window_set_event_compression (window, FALSE); Inkscape::Preferences *prefs = Inkscape::Preferences::get(); if (prefs->getBool("/options/useextinput/value", true)) { -- GitLab From 09f55021be0ad36a1f70e33326401a70de4e6903 Mon Sep 17 00:00:00 2001 From: Yale Zhang Date: Wed, 3 Jan 2018 22:55:06 -0800 Subject: [PATCH 2/6] This solves the jerky redraw when dragging objects (or no redraw when dragging quickly). There are 4 events involved for dragging it seems: 1. GDKEventMouse 2. selection modified generated in Selection::_scheduled_modified() with priority 101 3. redraw generated in SPCanvas::addIdle() with priority 200 calls SPCanvas::paint() which only redraws to *offscreen* buffer 4. refresh/expose generated in gtk_widget_queue_draw_area() with priority 120 calls SPCanvas::handle_draw() which actually updates the screen For fast redraw response, you want to execute #2, then #3, then #4 as soon as possible. This requires the priorities for each event to be higher than the previous or else mouse events that come later will get processed before earlier redraw,expose events already in the queue. Since this is real time stuff, the longer an event waits in the queue, the higher its priority should be, so this is a priority inversion. What I don't understand is: 1. why this problem didn't happen in 0.92 (GTK2) 2. why separate redraw and expose, which adds latency It seems it's to avoid tearing since redraw can take longer than the monitor refresh interval But why not use double buffering and swap buffers immediately after redraw - that seems faster than generating an expose event --- src/display/sp-canvas.cpp | 3 ++- src/selection.cpp | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/display/sp-canvas.cpp b/src/display/sp-canvas.cpp index ffcfe12ba6..35ec729230 100644 --- a/src/display/sp-canvas.cpp +++ b/src/display/sp-canvas.cpp @@ -145,7 +145,8 @@ struct SPCanvasClass { namespace { -gint const UPDATE_PRIORITY = G_PRIORITY_DEFAULT_IDLE; +gint const UPDATE_PRIORITY = GDK_PRIORITY_REDRAW + 10; // must be lower priority than GDK_PRIORITY_REDRAW or else redraw (handle_draw()) might be very jerky or not happen at all + GdkWindow *getWindow(SPCanvas *canvas) { diff --git a/src/selection.cpp b/src/selection.cpp index 65e0e1e973..976d477063 100644 --- a/src/selection.cpp +++ b/src/selection.cpp @@ -30,7 +30,7 @@ #include "desktop.h" #include "document.h" -#define SP_SELECTION_UPDATE_PRIORITY (G_PRIORITY_HIGH_IDLE + 1) +#define SP_SELECTION_UPDATE_PRIORITY (GDK_PRIORITY_REDRAW + 20) // must be lower priority than UPDATE_PRIORITY in sp-canvas.cpp or else there will be no redraw when dragging namespace Inkscape { -- GitLab From 93d9a940376337ef3dd79a3de3cc850a9f9edca6 Mon Sep 17 00:00:00 2001 From: Yale Zhang Date: Tue, 6 Feb 2018 03:52:28 -0800 Subject: [PATCH 3/6] my temp compile changes --- src/2geom/math-utils.h | 2 +- src/libvpsc/isnan.h | 2 +- src/main.cpp | 9 +++++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/2geom/math-utils.h b/src/2geom/math-utils.h index 0d5d3667bd..af43993c98 100644 --- a/src/2geom/math-utils.h +++ b/src/2geom/math-utils.h @@ -103,7 +103,7 @@ inline void sincos(double angle, double &sin_, double &cos_) { * the C++ standard (which predates C99). */ -#if defined(__isnan) +#if 1//defined(__isnan) # define IS_NAN(_a) (__isnan(_a)) #elif defined(__APPLE__) && __GNUC__ == 3 # define IS_NAN(_a) (__isnan(_a)) /* MacOSX/Darwin definition < 10.4 */ diff --git a/src/libvpsc/isnan.h b/src/libvpsc/isnan.h index 1e32b8a7a4..80967d68b6 100644 --- a/src/libvpsc/isnan.h +++ b/src/libvpsc/isnan.h @@ -43,7 +43,7 @@ #include #include -#if defined(__isnan) +#if 1//defined(__isnan) # define isNaN(_a) (__isnan(_a)) /* MacOSX/Darwin definition < 10.4 */ #elif defined(WIN32) || defined(_isnan) # define isNaN(_a) (_isnan(_a)) /* Win32 definition */ diff --git a/src/main.cpp b/src/main.cpp index 94e63ec1c0..13be5faa4e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -681,6 +681,15 @@ static void set_extensions_env() int main(int argc, char **argv) { +#ifdef _WIN32 + BOOL success = AllocConsole(); + AttachConsole(GetCurrentProcessId()); + freopen ("CONOUT$", "w", stdout); + dup2 (fileno (stdout), 1); + freopen ("CONOUT$", "w", stderr); + dup2 (fileno (stderr), 2); +#endif + #ifdef HAVE_FPSETMASK /* This is inherited from Sodipodi code, where it was in #ifdef __FreeBSD__. It's probably safe to remove: the default mask is already 0 in C99, and in current FreeBSD according to -- GitLab From 79e74d8bfaec6a2b9b754a38f1656d8e8db71465 Mon Sep 17 00:00:00 2001 From: Yale Zhang Date: Fri, 16 Feb 2018 02:30:33 -0800 Subject: [PATCH 4/6] Send rendered canvas immediatly to screen in paint() instead of waiting for expose event (handle_draw()) This was the old way until bzr r14795 (Hackfest 2016: Fix SPCanvas to comply with GTK3 rendering model) This reduces latency by ~10ms --- src/display/sp-canvas.cpp | 69 +++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src/display/sp-canvas.cpp b/src/display/sp-canvas.cpp index 35ec729230..694f9a1006 100644 --- a/src/display/sp-canvas.cpp +++ b/src/display/sp-canvas.cpp @@ -145,8 +145,8 @@ struct SPCanvasClass { namespace { -gint const UPDATE_PRIORITY = GDK_PRIORITY_REDRAW + 10; // must be lower priority than GDK_PRIORITY_REDRAW or else redraw (handle_draw()) might be very jerky or not happen at all - +#define DIRECT_DRAW +gint const UPDATE_PRIORITY = GDK_PRIORITY_REDRAW + 10; // must be same or lower priority than GDK_PRIORITY_REDRAW or else canvas, rulers, & status bar will update at very low rate or not at all (they'll starve) GdkWindow *getWindow(SPCanvas *canvas) { @@ -1562,6 +1562,9 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect // Create a temporary surface that draws directly to _backing_store cairo_surface_flush(_backing_store); // cairo_surface_write_to_png( _backing_store, "debug0.png" ); +//#define METHOD2 +#ifndef METHOD2 + // cleaner, but not thread safe because of using cairo_surface_set_device_scale() unsigned char *data = cairo_image_surface_get_data(_backing_store); int stride = cairo_image_surface_get_stride(_backing_store); @@ -1585,8 +1588,17 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect buf.ct = cairo_create(imgs); +#else + cairo_surface_set_device_offset(_backing_store, paint_rect.left() - _x0, paint_rect.top() - _y0); + buf.ct = cairo_create(_backing_store); + cairo_rectangle(buf.ct, 0, 0, paint_rect.width(), paint_rect.height()); + cairo_clip(buf.ct); +#endif cairo_save(buf.ct); + +#ifndef METHOD2 cairo_translate(buf.ct, -paint_rect.left(), -paint_rect.top()); +#endif cairo_set_source(buf.ct, _background); cairo_set_operator(buf.ct, CAIRO_OPERATOR_SOURCE); cairo_paint(buf.ct); @@ -1612,6 +1624,7 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect transf = Inkscape::CMSSystem::getDisplayTransform(); } + #if 0 if (transf) { cairo_surface_flush(imgs); unsigned char *px = cairo_image_surface_get_data(imgs); @@ -1622,6 +1635,7 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect } cairo_surface_mark_dirty(imgs); } + #endif } #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2) @@ -1630,9 +1644,11 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect // Mark the painted rectangle clean markRect(paint_rect, 0); - + +#ifndef DIRECT_DRAW gtk_widget_queue_draw_area(GTK_WIDGET(this), paint_rect.left() -_x0, paint_rect.top() - _y0, paint_rect.width(), paint_rect.height()); +#endif } struct PaintRectSetup { @@ -1946,16 +1962,57 @@ int SPCanvas::paint() cairo_region_t *to_draw = cairo_region_create_rectangle(&crect); cairo_region_subtract(to_draw, _clean_region); + bool aborted = false; int n_rects = cairo_region_num_rectangles(to_draw); for (int i = 0; i < n_rects; ++i) { cairo_rectangle_int_t crect; cairo_region_get_rectangle(to_draw, i, &crect); if (!paintRect(crect.x, crect.y, crect.x + crect.width, crect.y + crect.height)) { // Aborted - cairo_region_destroy(to_draw); - return FALSE; + aborted = true; }; } +#ifdef DIRECT_DRAW + // copy backing store to window + GtkWidget *widget = GTK_WIDGET(this); + GdkWindow *window = gtk_widget_get_window(gtk_widget_get_toplevel(widget)); + cairo_rectangle_int_t drawBounds, // world coordinates + drawBoundsWin, drawBoundsTopWin; +//#define WHOLE_SCREEN +#ifdef WHOLE_SCREEN + drawBounds.x = _x0; + drawBounds.y = _y0; + drawBounds.width = allocation.width; + drawBounds.height = allocation.height; +#else + cairo_region_get_extents(to_draw, &drawBounds); +#endif + drawBoundsWin = drawBounds; + drawBoundsWin.x -= _x0; + drawBoundsWin.y -= _y0; + + drawBoundsTopWin = drawBoundsWin; + gtk_widget_translate_coordinates(widget, gtk_widget_get_toplevel(widget), drawBoundsWin.x, drawBoundsWin.y, &drawBoundsTopWin.x, &drawBoundsTopWin.y); + + cairo_region_t *_drawBoundsTopWin = cairo_region_create_rectangle(&drawBoundsTopWin); + + GdkDrawingContext *context = gdk_window_begin_draw_frame(window, _drawBoundsTopWin); + cairo_t *xct = gdk_drawing_context_get_cairo_context(context); + int canvasOffsetX = drawBoundsTopWin.x - drawBoundsWin.x, + canvasOffsetY = drawBoundsTopWin.y - drawBoundsWin.y; + + //printf("dboundswin=[%d %d %d %d] dboundsTopWin=[%d %d %d %d]\n", drawBoundsWin.x, drawBoundsWin.y, drawBoundsWin.width, drawBoundsWin.height, + // drawBoundsTopWin.x, drawBoundsTopWin.y, drawBoundsTopWin.width, drawBoundsTopWin.height); + cairo_rectangle(xct, drawBoundsTopWin.x, drawBoundsTopWin.y, drawBounds.width, drawBounds.height); // WTH? why aren't the coordinates relative to the update region passed to gdk_window_begin_draw_frame()? + cairo_clip(xct); + cairo_surface_set_device_offset(_backing_store, 0, 0); + cairo_set_source_surface(xct, _backing_store, canvasOffsetX, canvasOffsetY); + cairo_set_operator(xct, CAIRO_OPERATOR_SOURCE); + cairo_paint(xct); + + gdk_window_end_draw_frame(window, context); + cairo_region_destroy(_drawBoundsTopWin); +#endif // we've had a full unaborted redraw, reset the full redraw counter if (_forced_redraw_limit != -1) { @@ -1964,7 +2021,7 @@ int SPCanvas::paint() cairo_region_destroy(to_draw); - return TRUE; + return !aborted; } int SPCanvas::doUpdate() -- GitLab From 03bafd62897411978d6952cbfd06d7eb5bb32fe0 Mon Sep 17 00:00:00 2001 From: Yale Zhang Date: Fri, 16 Feb 2018 02:32:26 -0800 Subject: [PATCH 5/6] add latency measuring instrumentation --- src/display/sp-canvas.cpp | 80 ++++++++++++++++++++++++++++++++++++ src/ui/tools/select-tool.cpp | 7 ++++ 2 files changed, 87 insertions(+) diff --git a/src/display/sp-canvas.cpp b/src/display/sp-canvas.cpp index 694f9a1006..e18f3bba5b 100644 --- a/src/display/sp-canvas.cpp +++ b/src/display/sp-canvas.cpp @@ -1838,8 +1838,38 @@ void SPCanvas::endForcedFullRedraws() _forced_redraw_limit = -1; } +#ifdef _WIN32 + #include +#else + #include + using namespace std::chrono; +#endif + +uint64_t MyClock() +{ +#ifdef _WIN32 + // don't use system_clock() because it's hard to use system_clock in GTK which is C not C++ + FILETIME t; + GetSystemTimePreciseAsFileTime(&t); + return *(uint64_t *)&t; +#else + return system_clock::now().time_since_epoch().count(); +#endif +} + +uint64_t T_exposeBeginBurst; + gboolean SPCanvas::handle_draw(GtkWidget *widget, cairo_t *cr) { +#ifndef DIRECT_DRAW + uint64_t t = MyClock(); + bool newBurst = false; + if (t - T_exposeBeginBurst > 5000000) + { + T_exposeBeginBurst = t; + newBurst = true; + } +#endif SPCanvas *canvas = SP_CANVAS(widget); #if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 12, 0) @@ -1904,7 +1934,21 @@ gboolean SPCanvas::handle_draw(GtkWidget *widget, cairo_t *cr) { canvas->addIdle(); } cairo_region_destroy(dirty_region); +#ifndef DIRECT_DRAW + if (newBurst) + { + extern uint64_t T_lastDragBurst, T_addIdleBurst, + T_drawBeginBurst, T_drawEndBurst; + uint64_t t = MyClock(); + printf("idle=%llu drawBegin=%llu drawEnd=%llu exposeBegin=%llu exposeEnd=%llu\n", + T_addIdleBurst - T_lastDragBurst, + T_drawBeginBurst - T_lastDragBurst, + T_drawEndBurst - T_lastDragBurst, + T_exposeBeginBurst - T_lastDragBurst, + t - T_lastDragBurst); + } +#endif return TRUE; } @@ -1949,8 +1993,17 @@ gint SPCanvas::handle_focus_out(GtkWidget *widget, GdkEventFocus *event) } } +uint64_t T_drawBeginBurst, T_drawEndBurst; + int SPCanvas::paint() { + uint64_t t = MyClock(); + bool newBurst = false; + if (t - T_drawBeginBurst > 5000000) + { + T_drawBeginBurst = t; + newBurst = true; + } if (_need_update) { sp_canvas_item_invoke_update(_root, Geom::identity(), 0); _need_update = FALSE; @@ -1996,7 +2049,10 @@ int SPCanvas::paint() cairo_region_t *_drawBoundsTopWin = cairo_region_create_rectangle(&drawBoundsTopWin); + uint64_t t0 = MyClock(); GdkDrawingContext *context = gdk_window_begin_draw_frame(window, _drawBoundsTopWin); + uint64_t t1 = MyClock(); + uint64_t T_beginFrame = t1 - t0; cairo_t *xct = gdk_drawing_context_get_cairo_context(context); int canvasOffsetX = drawBoundsTopWin.x - drawBoundsWin.x, canvasOffsetY = drawBoundsTopWin.y - drawBoundsWin.y; @@ -2010,7 +2066,10 @@ int SPCanvas::paint() cairo_set_operator(xct, CAIRO_OPERATOR_SOURCE); cairo_paint(xct); + t0 = MyClock(); + uint64_t T_copy = t0 - t1; gdk_window_end_draw_frame(window, context); + uint64_t T_endFrame = MyClock() - t0; cairo_region_destroy(_drawBoundsTopWin); #endif @@ -2021,6 +2080,21 @@ int SPCanvas::paint() cairo_region_destroy(to_draw); + if (newBurst) + { + T_drawEndBurst = MyClock(); + #ifdef DIRECT_DRAW + extern uint64_t T_lastDragBurst, T_addIdleBurst; + printf("idle=%llu drawBegin=%llu drawEnd=%llu T_beginFrame=%llu T_endFrame=%llu T_copy=%llu\n", + T_addIdleBurst - T_lastDragBurst, + T_drawBeginBurst - T_lastDragBurst, + T_drawEndBurst - T_lastDragBurst, + T_beginFrame, + T_endFrame, + T_copy + ); + #endif + } return !aborted; } @@ -2064,8 +2138,14 @@ gint SPCanvas::idle_handler(gpointer data) return !ret; } +uint64_t T_addIdleBurst; + void SPCanvas::addIdle() { + uint64_t t = MyClock(); + if (t - T_addIdleBurst > 5000000) + T_addIdleBurst = t; + if (_idle_id == 0) { _idle_id = gdk_threads_add_idle_full(UPDATE_PRIORITY, idle_handler, this, NULL); } diff --git a/src/ui/tools/select-tool.cpp b/src/ui/tools/select-tool.cpp index ddc6e2881a..2c40e24dd5 100644 --- a/src/ui/tools/select-tool.cpp +++ b/src/ui/tools/select-tool.cpp @@ -55,6 +55,9 @@ #endif +uint64_t T_lastDragBurst; +uint64_t MyClock(); + using Inkscape::DocumentUndo; GdkPixbuf *handles[13]; @@ -543,6 +546,10 @@ bool SelectTool::root_handler(GdkEvent* event) { case GDK_MOTION_NOTIFY: { + uint64_t t = MyClock(); + if (t - T_lastDragBurst > 5000000) + T_lastDragBurst = t; + tolerance = prefs->getIntLimited("/options/dragtolerance/value", 0, 0, 100); if ((event->motion.state & GDK_BUTTON1_MASK) && !this->space_panning) { -- GitLab From fd2d6d7b8ab36ed8e93b39aeac03a01c8419ff85 Mon Sep 17 00:00:00 2001 From: Yale Zhang Date: Fri, 16 Feb 2018 02:34:12 -0800 Subject: [PATCH 6/6] multithreaded rendering --- CMakeScripts/DefineDependsandFlags.cmake | 10 +++- src/display/SharedMutexGuard.h | 29 ++++++++++++ src/display/drawing-group.cpp | 3 ++ src/display/drawing-item.cpp | 39 +++++++++++++++- src/display/drawing-item.h | 4 +- src/display/drawing.cpp | 4 +- src/display/drawing.h | 3 +- src/display/nr-filter-turbulence.cpp | 3 ++ src/display/nr-filter-turbulence.h | 4 +- src/display/nr-filter.cpp | 2 + src/display/nr-style.cpp | 17 +++++++ src/display/nr-style.h | 2 + src/display/sodipodi-ctrl.cpp | 4 +- src/display/sodipodi-ctrl.h | 12 +++-- src/display/sp-canvas.cpp | 59 ++++++++++++++++++++++-- src/display/sp-canvas.h | 2 + src/object/sp-item.cpp | 3 ++ src/object/sp-item.h | 2 + src/style.h | 4 +- 19 files changed, 184 insertions(+), 22 deletions(-) create mode 100644 src/display/SharedMutexGuard.h diff --git a/CMakeScripts/DefineDependsandFlags.cmake b/CMakeScripts/DefineDependsandFlags.cmake index ffaf833f07..7c5b868311 100644 --- a/CMakeScripts/DefineDependsandFlags.cmake +++ b/CMakeScripts/DefineDependsandFlags.cmake @@ -284,9 +284,15 @@ set(TRY_GTKSPELL ON) ${GTKSPELL3_LIBRARIES} ) -find_package(Boost 1.19.0 REQUIRED) +if(WIN32) + find_package(Boost 1.19.0 REQUIRED COMPONENTS filesystem system) + list(APPEND INKSCAPE_LIBS ${Boost_FILESYSTEM_LIBRARY} ${Boost_SYSTEM_LIBRARY}) +else() + find_package(Boost 1.19.0 REQUIRED COMPONENTS filesystem thread system) + list(APPEND INKSCAPE_LIBS ${Boost_FILESYSTEM_LIBRARY} ${Boost_THREAD_LIBRARY} ${Boost_SYSTEM_LIBRARY}) +endif() + list(APPEND INKSCAPE_INCS_SYS ${Boost_INCLUDE_DIRS}) -# list(APPEND INKSCAPE_LIBS ${Boost_LIBRARIES}) find_package(ASPELL) if(ASPELL_FOUND) diff --git a/src/display/SharedMutexGuard.h b/src/display/SharedMutexGuard.h new file mode 100644 index 0000000000..51a39a6f8d --- /dev/null +++ b/src/display/SharedMutexGuard.h @@ -0,0 +1,29 @@ +template +class SharedMutexGuard +{ +public: + SharedMutexGuard(boost::shared_mutex &l) : lock(l) + { + if (shared) + lock.lock_shared(); + else + lock.lock(); + locked = true; + } + ~SharedMutexGuard() + { + Unlock(); + } + void Unlock() + { + if (!locked) + return; + if (shared) + lock.unlock_shared(); + else + lock.unlock(); + locked = false; + } + bool locked; + boost::shared_mutex &lock; +}; diff --git a/src/display/drawing-group.cpp b/src/display/drawing-group.cpp index 6abce34a2f..4892825c2b 100644 --- a/src/display/drawing-group.cpp +++ b/src/display/drawing-group.cpp @@ -86,9 +86,12 @@ DrawingGroup::_updateItem(Geom::IntRect const &area, UpdateContext const &ctx, u return beststate; } +#include "SharedMutexGuard.h" + unsigned DrawingGroup::_renderItem(DrawingContext &dc, Geom::IntRect const &area, unsigned flags, DrawingItem *stop_at) { + //SharedMutexGuard _lock(this->lock); // not needed because DrawingItem::render() already locks - but is that always the case? if (stop_at == NULL) { // normal rendering for (ChildrenList::iterator i = _children.begin(); i != _children.end(); ++i) { diff --git a/src/display/drawing-item.cpp b/src/display/drawing-item.cpp index cc4673bc89..605c3d93ec 100644 --- a/src/display/drawing-item.cpp +++ b/src/display/drawing-item.cpp @@ -215,6 +215,7 @@ DrawingItem::isAncestorOf(DrawingItem *item) const void DrawingItem::appendChild(DrawingItem *item) { + lock.lock(); item->_parent = this; assert(item->_child_type == CHILD_ORPHAN); item->_child_type = CHILD_NORMAL; @@ -227,11 +228,13 @@ DrawingItem::appendChild(DrawingItem *item) // rely on the appended child being in the default non-updated state. // We set propagate to true, because the child might have descendants of its own. item->_markForUpdate(STATE_ALL, true); + lock.unlock(); } void DrawingItem::prependChild(DrawingItem *item) { + lock.lock(); item->_parent = this; assert(item->_child_type == CHILD_ORPHAN); item->_child_type = CHILD_NORMAL; @@ -239,12 +242,16 @@ DrawingItem::prependChild(DrawingItem *item) // See appendChild for explanation item->_state = STATE_ALL; item->_markForUpdate(STATE_ALL, true); + lock.unlock(); } +#include "SharedMutexGuard.h" + /// Delete all regular children of this item (not mask or clip). void DrawingItem::clearChildren() { + SharedMutexGuard<> _lock(this->lock); if (_children.empty()) return; _markForRendering(); @@ -340,7 +347,8 @@ DrawingItem::setCached(bool cached, bool persistent) { static const char *cache_env = getenv("_INKSCAPE_DISABLE_CACHE"); if (cache_env) return; - + + SharedMutexGuard<> myLock(this->lock); if (_cached_persistent && !persistent) return; @@ -482,6 +490,7 @@ DrawingItem::setStrokePattern(DrawingPattern *pattern) void DrawingItem::setZOrder(unsigned z) { + SharedMutexGuard<> _lock(this->lock); if (!_parent) return; ChildrenList::iterator it = _parent->_children.iterator_to(*this); @@ -524,6 +533,7 @@ DrawingItem::setItemBounds(Geom::OptRect const &bounds) void DrawingItem::update(Geom::IntRect const &area, UpdateContext const &ctx, unsigned flags, unsigned reset) { + SharedMutexGuard<> myLock(this->lock); bool render_filters = _drawing.renderFilters(); bool outline = _drawing.outline(); @@ -678,6 +688,7 @@ struct MaskLuminanceToAlpha { unsigned DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flags, DrawingItem *stop_at) { + SharedMutexGuard myLock(this->lock); // WARNING: can deadlock when calling recursive, child functions since bool outline = _drawing.outline(); bool render_filters = _drawing.renderFilters(); @@ -691,6 +702,7 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag // TODO convert outline rendering to a separate virtual function if (outline) { + myLock.Unlock(); _renderOutline(dc, area, flags); return RENDER_OK; } @@ -722,7 +734,12 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag // render from cache if possible if (_cached) { if (_cache) { + lock.unlock_shared(); // can't call upgrade or else deadlock + lock.lock(); _cache->prepare(); + lock.unlock(); + lock.lock_shared(); + set_cairo_blend_operator( dc, _mix_blend_mode ); _cache->paintFromCache(dc, carea); @@ -734,7 +751,13 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag Geom::OptIntRect cl = _drawing.cacheLimit(); cl.intersectWith(_drawbox); if (cl) { + // locking not really necessary since it's atomic, just to please ThreadSanitizer + // without it, there will be a whole bunch of false positives + lock.unlock_shared(); // can't call upgrade or else deadlock + lock.lock(); _cache = new DrawingCache(*cl, device_scale); + lock.unlock(); + lock.lock_shared(); } } } else { @@ -773,6 +796,7 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag // filters and opacity do not apply when rendering the ancestors of the filtered // element if ((flags & RENDER_FILTER_BACKGROUND) || !needs_intermediate_rendering) { + myLock.Unlock(); return _renderItem(dc, *carea, flags & ~RENDER_FILTER_BACKGROUND, stop_at); } @@ -824,7 +848,10 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag // 3. Render object itself ict.pushGroup(); + + lock.unlock_shared(); render_result = _renderItem(ict, *iarea, flags, stop_at); + lock.lock_shared(); // 4. Apply filter. if (_filter && render_filters) { @@ -849,7 +876,7 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag // the internals of the filter need to use cairo_get_group_target() // instead of cairo_get_target(). } - + // 5. Render object inside the composited mask + clip ict.popGroupToSource(); ict.setOperator(CAIRO_OPERATOR_IN); @@ -857,12 +884,20 @@ DrawingItem::render(DrawingContext &dc, Geom::IntRect const &area, unsigned flag // 6. Paint the completed rendering onto the base context (or into cache) if (_cached && _cache) { + // upgrade lock here because to because creating DrawingContext will mutate _cache (calls createRawContext) + lock.unlock_shared(); // can't call upgrade or else deadlock + lock.lock(); + { DrawingContext cachect(*_cache); cachect.rectangle(*carea); cachect.setOperator(CAIRO_OPERATOR_SOURCE); cachect.setSource(&intermediate); cachect.fill(); _cache->markClean(*carea); + // destructor needs to be called before unlocking + } + lock.unlock(); + lock.lock_shared(); } dc.rectangle(*carea); dc.setSource(&intermediate); diff --git a/src/display/drawing-item.h b/src/display/drawing-item.h index 21f6ffacc8..a6a5e7974a 100644 --- a/src/display/drawing-item.h +++ b/src/display/drawing-item.h @@ -19,6 +19,7 @@ #include #include #include +#include namespace Glib { class ustring; @@ -226,7 +227,8 @@ protected: unsigned _isolation : 1; unsigned _mix_blend_mode : 4; - + boost::shared_mutex lock; + friend class Drawing; }; diff --git a/src/display/drawing.cpp b/src/display/drawing.cpp index 71fb94be07..85527ea96c 100644 --- a/src/display/drawing.cpp +++ b/src/display/drawing.cpp @@ -44,7 +44,7 @@ Drawing::Drawing(SPCanvasArena *arena) , _grayscale_colormatrix(std::vector (grayscale_value_matrix, grayscale_value_matrix + 20 )) , _canvasarena(arena) { - + omp_init_lock(&lock); } Drawing::~Drawing() @@ -204,6 +204,7 @@ Drawing::pick(Geom::Point const &p, double delta, unsigned flags) void Drawing::_pickItemsForCaching() { + omp_set_lock(&lock); // we cache the objects with the highest score until the budget is exhausted _candidate_items.sort(std::greater()); size_t used = 0; @@ -228,6 +229,7 @@ Drawing::_pickItemsForCaching() for (std::set::iterator j = to_uncache.begin(); j != to_uncache.end(); ++j) { (*j)->setCached(false); } + omp_unset_lock(&lock); } } // end namespace Inkscape diff --git a/src/display/drawing.h b/src/display/drawing.h index e472c8f5b5..d25dce4883 100644 --- a/src/display/drawing.h +++ b/src/display/drawing.h @@ -12,6 +12,7 @@ #ifndef SEEN_INKSCAPE_DISPLAY_DRAWING_H #define SEEN_INKSCAPE_DISPLAY_DRAWING_H +#include #include <2geom/rect.h> #include #include @@ -102,7 +103,7 @@ private: Filters::FilterColorMatrix::ColorMatrixMatrix _grayscale_colormatrix; SPCanvasArena *_canvasarena; // may be NULL if this arena is not the screen // but used for export etc. - + omp_lock_t lock; friend class DrawingItem; }; diff --git a/src/display/nr-filter-turbulence.cpp b/src/display/nr-filter-turbulence.cpp index 349bcc2423..81a2f5b0d8 100644 --- a/src/display/nr-filter-turbulence.cpp +++ b/src/display/nr-filter-turbulence.cpp @@ -315,6 +315,7 @@ FilterTurbulence::FilterTurbulence() , fTileX(1) //guessed , fTileY(1) //guessed { + omp_init_lock(&lock); } FilterPrimitive * FilterTurbulence::create() { @@ -396,6 +397,7 @@ void FilterTurbulence::render_cairo(FilterSlot &slot) set_cairo_surface_ci(out, (SPColorInterpolation)_style->color_interpolation_filters.computed ); } + omp_set_lock(&lock); if (!gen->ready()) { Geom::Point ta(fTileX, fTileY); Geom::Point tb(fTileX + fTileWidth, fTileY + fTileHeight); @@ -403,6 +405,7 @@ void FilterTurbulence::render_cairo(FilterSlot &slot) Geom::Point(XbaseFrequency, YbaseFrequency), stitchTiles, type == TURBULENCE_FRACTALNOISE, numOctaves); } + omp_unset_lock(&lock); Geom::Affine unit_trans = slot.get_units().get_matrix_primitiveunits2pb().inverse(); Geom::Rect slot_area = slot.get_slot_area(); diff --git a/src/display/nr-filter-turbulence.h b/src/display/nr-filter-turbulence.h index 3960c2f8ef..8d0c38c2b6 100644 --- a/src/display/nr-filter-turbulence.h +++ b/src/display/nr-filter-turbulence.h @@ -22,7 +22,7 @@ */ #include <2geom/point.h> - +#include #include "display/nr-filter-primitive.h" #include "display/nr-filter-slot.h" #include "display/nr-filter-units.h" @@ -76,7 +76,7 @@ private: double fTileX; double fTileY; - + omp_lock_t lock; }; } /* namespace Filters */ diff --git a/src/display/nr-filter.cpp b/src/display/nr-filter.cpp index 4782f3f54a..a91955d581 100644 --- a/src/display/nr-filter.cpp +++ b/src/display/nr-filter.cpp @@ -108,6 +108,7 @@ int Filter::render(Inkscape::DrawingItem const *item, DrawingContext &graphic, D return 1; } Inkscape::Preferences *prefs = Inkscape::Preferences::get(); + //YZ: setFilter/BlurQuality are race conditions that can be ignored item->drawing().setFilterQuality(prefs->getInt("/options/filterquality/value", 0)); item->drawing().setBlurQuality(prefs->getInt("/options/blurquality/value", 0)); FilterQuality const filterquality = (FilterQuality)item->drawing().filterQuality(); @@ -232,6 +233,7 @@ Geom::OptRect Filter::filter_effect_area(Geom::OptRect const &bbox) /* TODO: fetch somehow the object ex and em lengths */ // Update for em, ex, and % values + // YZ: unresolved race condition - need to make this private to each thread _region_x.update(12, 6, len_x); _region_y.update(12, 6, len_y); _region_width.update(12, 6, len_x); diff --git a/src/display/nr-style.cpp b/src/display/nr-style.cpp index 31bb277552..69340c42ed 100644 --- a/src/display/nr-style.cpp +++ b/src/display/nr-style.cpp @@ -75,6 +75,7 @@ NRStyle::NRStyle() , font_size(0) { paint_order_layer[0] = PAINT_ORDER_NORMAL; + omp_init_lock(&lock); } NRStyle::~NRStyle() @@ -341,8 +342,23 @@ void NRStyle::set(SPStyle *style, SPStyle *context_style) update(); } +class LockGuard +{ +public: + LockGuard(omp_lock_t &l) : lock(l) + { + omp_set_lock(&lock); + } + ~LockGuard() + { + omp_unset_lock(&lock); + } + omp_lock_t &lock; +}; + bool NRStyle::prepareFill(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox, Inkscape::DrawingPattern *pattern) { + LockGuard _lock(this->lock); // update fill pattern if (!fill_pattern) { switch (fill.type) { @@ -407,6 +423,7 @@ void NRStyle::applyTextDecorationFill(Inkscape::DrawingContext &dc) bool NRStyle::prepareStroke(Inkscape::DrawingContext &dc, Geom::OptRect const &paintbox, Inkscape::DrawingPattern *pattern) { + LockGuard _lock(this->lock); if (!stroke_pattern) { switch (stroke.type) { case PAINT_SERVER: diff --git a/src/display/nr-style.h b/src/display/nr-style.h index 6c652311a6..617b1c59f6 100644 --- a/src/display/nr-style.h +++ b/src/display/nr-style.h @@ -15,6 +15,7 @@ #include #include <2geom/rect.h> +#include #include "color.h" class SPPaintServer; @@ -122,6 +123,7 @@ struct NRStyle { float font_size; int text_direction; + omp_lock_t lock; }; #endif diff --git a/src/display/sodipodi-ctrl.cpp b/src/display/sodipodi-ctrl.cpp index 04ec947f60..ed4bedd75e 100644 --- a/src/display/sodipodi-ctrl.cpp +++ b/src/display/sodipodi-ctrl.cpp @@ -210,6 +210,7 @@ sp_ctrl_init (SPCtrl *ctrl) ctrl->pixbuf = NULL; ctrl->_point = Geom::Point(0,0); + omp_init_lock(&ctrl->lock); } static void sp_ctrl_destroy(SPCanvasItem *object) @@ -584,10 +585,11 @@ sp_ctrl_render (SPCanvasItem *item, SPCanvasBuf *buf) if ((!ctrl->filled) && (!ctrl->stroked)) return; // the control-image is rendered into ctrl->cache + omp_set_lock(&ctrl->lock); if (!ctrl->build) { sp_ctrl_build_cache (ctrl, buf->device_scale); } - + omp_unset_lock(&ctrl->lock); // Must match width/height sp_ctrl_build_cache. int w = (ctrl->width * 2 + 1) * buf->device_scale; int h = (ctrl->height * 2 + 1) * buf->device_scale; diff --git a/src/display/sodipodi-ctrl.h b/src/display/sodipodi-ctrl.h index ac5ac9442d..78d7bea48b 100644 --- a/src/display/sodipodi-ctrl.h +++ b/src/display/sodipodi-ctrl.h @@ -7,6 +7,7 @@ * */ +#include #include #include "sp-canvas-item.h" #include "enums.h" @@ -40,11 +41,11 @@ struct SPCtrl : public SPCanvasItem { SPAnchorType anchor; gint width; gint height; - guint defined : 1; - guint shown : 1; - guint build : 1; - guint filled : 1; - guint stroked : 1; + bool defined; + bool shown; + bool build; + bool filled; + bool stroked; guint32 fill_color; guint32 stroke_color; gdouble angle; @@ -55,6 +56,7 @@ struct SPCtrl : public SPCanvasItem { void moveto(Geom::Point const p); Geom::Point _point; + omp_lock_t lock; }; struct SPCtrlClass : public SPCanvasItemClass{ diff --git a/src/display/sp-canvas.cpp b/src/display/sp-canvas.cpp index e18f3bba5b..ce611c012a 100644 --- a/src/display/sp-canvas.cpp +++ b/src/display/sp-canvas.cpp @@ -20,7 +20,7 @@ #ifdef HAVE_CONFIG_H # include #endif - +#include #include #include #include @@ -133,7 +133,7 @@ struct SPCanvasGroup { SPCanvasItem item; std::list items; - + boost::shared_mutex lock; }; /** @@ -769,6 +769,7 @@ static void sp_canvas_group_class_init(SPCanvasGroupClass *klass) static void sp_canvas_group_init(SPCanvasGroup * group) { new (&group->items) std::list; + new (&group->lock) boost::shared_mutex; } void SPCanvasGroup::destroy(SPCanvasItem *object) @@ -792,7 +793,10 @@ void SPCanvasGroup::destroy(SPCanvasItem *object) void SPCanvasGroup::update(SPCanvasItem *item, Geom::Affine const &affine, unsigned int flags) { - SPCanvasGroup const *group = SP_CANVAS_GROUP(item); + SPCanvasGroup *group = SP_CANVAS_GROUP(item); + // must not render when in update + group->lock.lock(); + Geom::OptRect bounds; for (std::list::const_iterator it = group->items.begin(); it != group->items.end(); ++it) { @@ -815,6 +819,7 @@ void SPCanvasGroup::update(SPCanvasItem *item, Geom::Affine const &affine, unsig // FIXME ? item->x1 = item->x2 = item->y1 = item->y2 = 0; } + group->lock.unlock(); } double SPCanvasGroup::point(SPCanvasItem *item, Geom::Point p, SPCanvasItem **actual_item) @@ -863,8 +868,10 @@ double SPCanvasGroup::point(SPCanvasItem *item, Geom::Point p, SPCanvasItem **ac void SPCanvasGroup::render(SPCanvasItem *item, SPCanvasBuf *buf) { - SPCanvasGroup const *group = SP_CANVAS_GROUP(item); - + SPCanvasGroup *group = SP_CANVAS_GROUP(item); + // protect it from SPCanvasGroup::update(), called by idle_handler -> doUpdate + group->lock.lock_shared(); + for (std::list::const_iterator it = group->items.begin(); it != group->items.end(); ++it) { SPCanvasItem *child = *it; if (child->visible) { @@ -878,6 +885,7 @@ void SPCanvasGroup::render(SPCanvasItem *item, SPCanvasBuf *buf) } } } + group->lock.unlock_shared(); } void SPCanvasGroup::viewboxChanged(SPCanvasItem *item, Geom::IntRect const &new_area) @@ -981,6 +989,7 @@ static void sp_canvas_init(SPCanvas *canvas) canvas->_enable_cms_display_adj = false; new (&canvas->_cms_key) Glib::ustring(""); #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2) + omp_init_lock(&canvas->lock); } void SPCanvas::shutdownTransients() @@ -1535,6 +1544,8 @@ int SPCanvas::handle_motion(GtkWidget *widget, GdkEventMotion *event) return status; } +uint64_t MyClock(); + void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect const &canvas_rect, int /*sw*/) { @@ -1590,7 +1601,9 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect #else cairo_surface_set_device_offset(_backing_store, paint_rect.left() - _x0, paint_rect.top() - _y0); + omp_set_lock(&lock); // cairo_create mutates _backing_store buf.ct = cairo_create(_backing_store); + omp_unset_lock(&lock); cairo_rectangle(buf.ct, 0, 0, paint_rect.width(), paint_rect.height()); cairo_clip(buf.ct); #endif @@ -1611,7 +1624,9 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect // cairo_surface_write_to_png( imgs, "debug2.png" ); // output to X + omp_set_lock(&lock); // cairo_destroy mutates _backing_store cairo_destroy(buf.ct); + omp_unset_lock(&lock); #if defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2) if (_enable_cms_display_adj) { @@ -1643,12 +1658,14 @@ void SPCanvas::paintSingleBuffer(Geom::IntRect const &paint_rect, Geom::IntRect // cairo_surface_write_to_png( _backing_store, "debug3.png" ); // Mark the painted rectangle clean + omp_set_lock(&lock); markRect(paint_rect, 0); #ifndef DIRECT_DRAW gtk_widget_queue_draw_area(GTK_WIDGET(this), paint_rect.left() -_x0, paint_rect.top() - _y0, paint_rect.width(), paint_rect.height()); #endif + omp_unset_lock(&lock); } struct PaintRectSetup { @@ -1700,6 +1717,28 @@ int SPCanvas::paintRectInternal(PaintRectSetup const *setup, Geom::IntRect const if (bw * bh < setup->max_pixels) { // We are small enough + #if 1 + // YZ - multithread + using namespace std::chrono; + int idealBlocks = omp_get_num_procs(); // could use more to improve load balancing, but the extra cost of iterating through the scene and additional per polygon setup will probably offset this (TODO: ignore hyper threaded cores) + int blockSize = (this_rect.height() + idealBlocks - 1) / idealBlocks; //64 use small size to test for race conditions + // Always paint towards the mouse first + auto t0 = MyClock(); + #pragma omp parallel + { + #pragma omp for schedule(static) + for (int y = this_rect.top(); y < this_rect.bottom(); y += blockSize) + { + Geom::IntRect r(this_rect.left(), y, this_rect.right(), std::min(y + blockSize, this_rect.bottom())); + paintSingleBuffer(r, setup->canvas_rect, bw); + } + #if 0 + // 5 ms delay on Windows! + #pragma omp critical + printf("t_thread%d=%llu\n", omp_get_thread_num(), MyClock() - t0); + #endif + } + #else /* GdkRectangle r; r.x = this_rect.x0 - setup->canvas->x0; @@ -1713,6 +1752,7 @@ int SPCanvas::paintRectInternal(PaintRectSetup const *setup, Geom::IntRect const paintSingleBuffer(this_rect, setup->canvas_rect, bw); //gdk_window_end_paint(window); + #endif return 1; } @@ -1995,8 +2035,11 @@ gint SPCanvas::handle_focus_out(GtkWidget *widget, GdkEventFocus *event) uint64_t T_drawBeginBurst, T_drawEndBurst; +bool renderLoop; int SPCanvas::paint() { + using namespace std::chrono; +start: uint64_t t = MyClock(); bool newBurst = false; if (t - T_drawBeginBurst > 5000000) @@ -2095,6 +2138,12 @@ int SPCanvas::paint() ); #endif } + + if (renderLoop) + { + dirtyRect(Geom::IntRect(_x0, _y0, _x0 + allocation.width, _y0 + allocation.height)); + goto start; + } return !aborted; } diff --git a/src/display/sp-canvas.h b/src/display/sp-canvas.h index d30e0a485b..2cbb5c0ef6 100644 --- a/src/display/sp-canvas.h +++ b/src/display/sp-canvas.h @@ -25,6 +25,7 @@ #endif #include +#include #include #include #include <2geom/affine.h> @@ -247,6 +248,7 @@ public: #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2) bool _is_scrolling; + omp_lock_t lock; }; bool sp_canvas_world_pt_inside_window(SPCanvas const *canvas, Geom::Point const &world); diff --git a/src/object/sp-item.cpp b/src/object/sp-item.cpp index f7a4ff6720..187f888d85 100644 --- a/src/object/sp-item.cpp +++ b/src/object/sp-item.cpp @@ -101,6 +101,7 @@ SPItem::SPItem() : SPObject() { style->signal_stroke_ps_changed.connect(sigc::bind(sigc::ptr_fun(stroke_ps_ref_changed), this)); avoidRef = new SPAvoidRef(this); + omp_init_lock(&lock); } SPItem::~SPItem() { @@ -1184,6 +1185,7 @@ void SPItem::hide(unsigned int /*key*/) { void SPItem::invoke_hide(unsigned key) { + omp_set_lock(&lock); this->hide(key); SPItemView *ref = NULL; @@ -1219,6 +1221,7 @@ void SPItem::invoke_hide(unsigned key) } v = next; } + omp_unset_lock(&lock); } // Adjusters diff --git a/src/object/sp-item.h b/src/object/sp-item.h index 36af02edc7..4bc4b795da 100644 --- a/src/object/sp-item.h +++ b/src/object/sp-item.h @@ -31,6 +31,7 @@ //class SPGuideConstraint; #include "sp-guide-constraint.h" +#include class SPClipPathReference; class SPMaskReference; @@ -401,6 +402,7 @@ public: virtual void convert_to_guides() const; virtual int event(SPEvent *event); + omp_lock_t lock; }; diff --git a/src/style.h b/src/style.h index 1b6ee2f479..69ca5ab675 100644 --- a/src/style.h +++ b/src/style.h @@ -59,8 +59,8 @@ public: void mergeString( char const *const p ); bool operator==(const SPStyle& rhs); - int style_ref() { ++_refcount; return _refcount; } - int style_unref() { --_refcount; return _refcount; } + int style_ref() { __sync_fetch_and_add(&_refcount, 1); return _refcount; } // Y.Z. is this needed any more after fixing other more fundamental race conditions? + int style_unref() { __sync_fetch_and_add(&_refcount, -1); return _refcount; } int refCount() { return _refcount; } private: -- GitLab