Created attachment 148683 [details] writer chart update demo steps to reproduce: - open attached writer document - change table cell value e.g. B2 from 1000 to 500 -> nothing happens - Menu Tools/Update/Chart -> nothing happens - double click chart -> chart will refresh with LO 4.4.7.2 chart will update after cell value has changed
Confirm, Version: 6.3.0.0.alpha0+ Build ID: 3fa4674615b747e219afe5bf0a9b689df3840439 CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3;
This seems to have begun at the below commit. Adding Cc: to Armin Le Grand; Could you possibly take a look at this one? Thanks There are only 'skip'ped commits left to test. The first bad commit could be any of: 1) 155c1848ff10ebc2d4002ffa249d66ea7a6a99fc 2) e091e6ede77ea72eeb186cc6d9a076de668e52e3 3) 9edb52139442b9417292fad3366fe86387040d89 1) Commit 155c1848ff10ebc2d4002ffa249d66ea7a6a99fc (refs/bisect/skip-155c1848ff10ebc2d4002ffa249d66ea7a6a99fc) Author: Jenkins Build User <tdf@pollux.tdf> Date: Wed Sep 28 08:55:25 2016 +0200 source 9f0766917a4fb1bc8fe1786c3b46132dd63c1c66 source 9f0766917a4fb1bc8fe1786c3b46132dd63c1c66 source de7d596d116b5231bff000a57be3fae481744bab source 64e1113916a6b19b30f95b454018528571ac84df author Armin Le Grand <Armin.Le.Grand@cib.de> 2016-07-01 14:40:00 +0200 committer Thorsten Behrens <Thorsten.Behrens@CIB.de> 2016-07-07 22:32:38 +0200 commit 9f0766917a4fb1bc8fe1786c3b46132dd63c1c66 (patch) tree a38af5308785c8735d4e2907b01d16a3c8c74814 parent de7d596d116b5231bff000a57be3fae481744bab (diff) tdf#50613 add support to load charts asynchronously 2) commit e091e6ede77ea72eeb186cc6d9a076de668e52e3 (refs/bisect/skip-e091e6ede77ea72eeb186cc6d9a076de668e52e3) Author: Jenkins Build User <tdf@pollux.tdf> Date: Wed Sep 28 08:55:47 2016 +0200 source 68e8d075d92ae4002898a4665a9d7c50162c2511 author Armin Le Grand <Armin.Le.Grand@cib.de> 2016-07-01 14:50:00 +0200 committer Thorsten Behrens <Thorsten.Behrens@CIB.de> 2016-07-07 22:32:39 +0200 commit 68e8d075d92ae4002898a4665a9d7c50162c2511 (patch) tree a1cead2a8026021c7dcf81c88ee8feec1ba0396b parent 9f0766917a4fb1bc8fe1786c3b46132dd63c1c66 (diff) sw: tdf#50613 fix waitFinished into a loop 3) commit 9edb52139442b9417292fad3366fe86387040d89 (HEAD, refs/bisect/bad) Author: Jenkins Build User <tdf@pollux.tdf> Date: Wed Sep 28 08:56:22 2016 +0200 source 3dc8ee7d8eec40093af5df3113ef226bc59220ff author Armin Le Grand <Armin.Le.Grand@cib.de> 2016-07-01 15:10:00 +0200 committer Thorsten Behrens <Thorsten.Behrens@CIB.de> 2016-07-07 22:32:39 +0200 commit 3dc8ee7d8eec40093af5df3113ef226bc59220ff (patch) tree 75eaa5c4add4155192f3b5d0d722ec48f4eb9ce7 parent 68e8d075d92ae4002898a4665a9d7c50162c2511 (diff) sw: tdf#50613 fix async chart load handling
See https://bugs.documentfoundation.org/show_bug.cgi?id=124221#c7 Closing as a duplicate of bug 124221 *** This bug has been marked as a duplicate of bug 124221 ***
Now we are talking about a real regression. Regardless of the new caching mechanism, Tools - Update - Charts should trigger an update.
Charts also do not update when menu item Tools-> Update -> Charts is selected. This menu item should work or be removed.
Taking a look...
Happens as described. Checking buffered/pre-loaded Chart OLE SdrObject...
Checked the suggested commits 9f0766917a4fb1bc8fe1786c3b46132dd63c1c66 68e8d075d92ae4002898a4665a9d7c50162c2511 3dc8ee7d8eec40093af5df3113ef226bc59220ff these all have to do with a feature that allows faster load by loading the metafile visualization of a Chart OLE, while starting to load the actual chart in BG. This is controlled by bAsynchronousLoadingAllowed which is currently // disabled for now, need to check deeper static bool bAsynchronousLoadingAllowed = false; // loplugin:constvars:ignore false and thus the whole mechanism disabled. That means the above commits do not really have to do with the missing refresh. I think it's more that many OLE reworks that happened over time (the three commits above are from 2016-07-07), so *very* old. So not a regression from that commits, but I'll take a look...
Checked to set bAsynchronousLoadingAllowed to true. You can see the effect by the Chart 'changing' slightly after 1st paint, so that mechanism works. I have no idea why it is switched off...
Found SwDoc::UpdateCharts_ && SwDoc::UpdateCharts in sw/source/core/doc/docchart.cxx. These *get* called by either changing a number and leaving the cell (loseFocus) or by tools/update/chart. There is a comment there: // following this the framework will now take care of repainting // the chart or it's replacement image... Exactly this does not happen/work. I am not a framework specialist, but indeed that should happen. What I know better is the primitive side of things, so somehow SwOLEObj::resetBufferedData() needs o be called - however. It would even be better to have some ViewContac/ViewObjectContact/ObjectContact place here that would be able to check in VOC if chart did change and decides if primitives for visualization can/will bne reused (to keep decompositions & bufferings from chart) Trying to find out why framework does not do it's job here...
There is a lot of invalidation/messaging going on, I debugged into it. None of these leads to SwOLEObj::resetBufferedData() being called. Unfortunately SwOLEObj is no (yet) added to the VC/VOC/OC mechanism, grep for tryToGetChartContentAsPrimitive2DSequence to see how in comparison SdrOle2Obj creates needed ViewContactOfSdrOle2Obj && ViewObjectContactOfSdrOle2Obj. So I will have to find a way to use one of the notifying paths to do that. Notifyinig e.g. can be seen at - ChartModel::modified - ChartView::modified - ChartView::impl_notifyModeChangeListener - * SfxInPlaceClient_Impl::notifyEvent - ChartView::impl_updateView where SfxInPlaceClient_Impl::notifyEvent i sthe best candidate eventually to add this. It does m_pClient->FormatChanged(); m_pClient->ViewChanged(); m_pClient->Invalidate(); which is closest to the needed Invalidate. Let's see if we can get to the needed SwOLEObj there...
To see how this is done in the other apps I tried to create ssth similar in draw/impress, but failed. I can insert a table, fill it & select all. But inserting a Chart then inserts the standard-chart. Wouldn't that be a nice feature...? Doing it in calc failed due to calc immediately crashing in current master - sigh... So I will continue to see if I can do this in Writer with given file, adding to one/some of the message passing stuff. dding fill VC/VOC/OC would be too much now for a fix - that will be part of getting SW further to primitive usage...
After quite some experiments to do this in one of he mentioned refreshes I decided that for now it will be easiest to do it directly inside SwDoc::UpdateCharts_. We needs this as long as the mechanism is incomplete (VC/VOC/OC in SW), so direct refresh is currently needed.
Solution is on https://gerrit.libreoffice.org/c/core/+/128572. I noticed that sometimes the very 1st change in the yellow-marked number does fail. I experimented with moving change in SwDoc::UpdateCharts_ before/after notifies, but changes nothing. Also tried to split. Also debugged to make sure SwDoc::UpdateCharts_ gets called at 1st change a all - it gets. When I do menu tools/update/charts (that always works, also after 1st change) it gets updated after change of 1st number. This *seems* to initialize something that is not up-to-date after load & 1st change. Still, proposed solution is better than current state, but I'll check and try to identify...
Seems I found the problem. At the time the chart gets repainted, the chart model is not yet invalidated, break at ChartView::impl_updateView line 2447 where false == m_bViewDirty. That gets set to true *later* from SwChartLockController_Helper::LockUnlockAllCharts that again is timer-controlled/triggered. This is probably to not always change the LockUnlock of charts, so done time-shifted using a timer callback. This means that all that updates triggered from SwDoc::UpdateCharts_ calling InvalidateTable will happen in XChartDocument later, thus SwOLEObj::tryToGetChartContentAsPrimitive2DSequence *will* get back the *same* outdated visualization - because the repaint is *faster* then the timer timeout that sets the chart to dirty. Next changes work since the *last* invalidate of chart data is then diffundated through to the chart and used then. Thus only the first refresh does not work - sigh... Thinking about how that could be interrupted.
Tried to use SwOleClient::FormatChanged() which gets triggered (over quite some indirections) from InvalidateTable in SwDoc::UpdateCharts_. What I would need there is to get to the SwOLEObj represented by that SwOleClient, but I found no possibility (yet). Looking further...
Armin Le Grand (Allotropia) committed a patch related to this issue. It has been pushed to "master": https://git.libreoffice.org/core/commit/d769e75de28a1afbb1df31b48840626cb35ed7ba tdf#122995 Trigger Chart refresh directly in UpdateCharts for SW It will be available in 7.4.0. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
*** Bug 124221 has been marked as a duplicate of this bug. ***
Now it works fine, verified in Version: 7.4.0.0.alpha0+ (x64) / LibreOffice Community Build ID: e27a41a362bf25e12487b36f625985b35fb891e3 CPU threads: 4; OS: Windows 6.1 Service Pack 1 Build 7601; UI render: Skia/Raster; VCL: win Locale: ru-RU (ru_RU); UI: ru-RU Calc: CL Armin will you plan to backport it to 7.3 and 7.2?
(In reply to Roman Kuznetsov from comment #19) > Armin will you plan to backport it to 7.3 and 7.2? Initialized...
Armin Le Grand (Allotropia) committed a patch related to this issue. It has been pushed to "libreoffice-7-2": https://git.libreoffice.org/core/commit/eea37aa26932d06ed8e93d001862bf45175c4446 tdf#122995 Trigger Chart refresh directly in UpdateCharts for SW It will be available in 7.2.6. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Armin Le Grand (Allotropia) committed a patch related to this issue. It has been pushed to "libreoffice-7-3": https://git.libreoffice.org/core/commit/310d196d7df37921e71f500869fca68cf00797d5 tdf#122995 Trigger Chart refresh directly in UpdateCharts for SW It will be available in 7.3.1. The patch should be included in the daily builds available at https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: https://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.