CollaboraOnline / online

Collabora Online is a collaborative online office suite based on LibreOffice technology. This is also the source for the Collabora Office apps for iOS and Android.
https://collaboraonline.com
Other
1.78k stars 681 forks source link

IOS App: Pages are not loaded in longer document #6355

Open scroom opened 1 year ago

scroom commented 1 year ago

Describe the bug Unfortunately, pages in a longer document are no longer loaded after a few pages, but only blank pages are displayed.

To Reproduce Steps to reproduce the behavior:

  1. Open a longer document
  2. Scroll through the document

Expected behavior All pages should be shown.

Actual behavior As described above. Only blank pages are shown. Screenshots I will upload some files in which I discovered the problem.

Desktop (please complete the following information)

plubius commented 1 year ago

I cannot reproduce this bug with the lastest TestFlight build (22.05.13.1) but it sounds very similar to issue #5876 which was fixed aft 22.05.12.4. Do you still see this bug in the TestFlight build?

If you don't already have access to the TestFlight build, you can access it using the following link:

https://testflight.apple.com/join/TEnBWi68

scroom commented 1 year ago

Unfortunately, I cannot install software myself, as it is a service device that is centrally managed.

mmeeks commented 1 year ago

Thanks Patrick - I guess this is probably a duplicate of: https://github.com/CollaboraOnline/online/issues/6369 at root, possibly @caolanm or @aszucs3 will get some time post 23.05 release to look at this.

plubius commented 1 year ago

Thanks Patrick - I guess this is probably a duplicate of: #6369 at root, possibly @caolanm or @aszucs3 will get some time post 23.05 release to look at this.

I submitted pull request with a partial fix for that bug for 22.05 in pull request #6358 a week or two ago. Would it be possible for Collabora to consider merging my pull request into 22.05 and doing a 22.05 iOS TestFlight/release in the interim while we wait for a 23.05 iOS release?

I admit the pull request only fixes issue #6369 for the subset of cases that I have seen on iOS (i.e. this bug and issue #5876) but IMHO a partial fix is better than the current state of no fix in the released version. Note: A more expansive attempt at fixing issue #6869 is in the current TestFlight build but that was found to cause some screen refresh problems in Calc. So, pull request #6958 removes the more aggressive canvas reclaiming code and leaves just enough reclaiming to fix these two bugs.

plubius commented 12 months ago

Collabora recently released 23.05.4.1 in the App Store. With that version, I now see a crash when scrolling to the bottom of @scroom's 2 documents. I will see if I can reproduce and debug this crash in a local build.

plubius commented 12 months ago

It looks like the crash is because we are hitting an assert(). Seems that this assert() is only hit once immediately after the first downward swipe or page down. Unfortunately, even with the debug patch, this bug still occurs when the documents are in read-only mode so more debugging will be needed:

diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index e334bf237b..16302c0820 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -3397,7 +3397,7 @@ void DocumentBroker::sendRequestedTiles(const std::shared_ptr& se
         {
             TileCombined newTileCombined = TileCombined::create(tilesNeedsRendering);

-            assert(!newTileCombined.hasDuplicates());
+            // assert(!newTileCombined.hasDuplicates());

             // Forward to child to render.
             const std::string req = newTileCombined.serialize("tilecombine");
mmeeks commented 12 months ago

Ooh - that's quite important; if we have duplicate tiles (IIRC) we not only waste time rendering duplicates, but we can (or perhaps I forget) have two threads operating on the same memory at once as we RLE/compress and delta them causing crashes in the Kit. I wonder how we end up there - what is in the string version of the tilecombine ? =) Thanks Patrick!

plubius commented 12 months ago

Here is the output of newTileCombined.serialize("tilecombine"):

tilecombine nviewid=1000 part=0 width=256 height=256 tileposx=7680,9600,11520,13440,15360,0,1920,3840,5760,7680,9600,0,1920,3840,5760,7680,9600,11520,13440,15360,0,1920,3840,5760,7680,9600,11520,13440,15360,0,1920 tileposy=1920,1920,1920,1920,1920,3840,3840,3840,3840,3840,3840,0,0,0,0,0,0,0,0,0,1920,1920,1920,1920,1920,1920,1920,1920,1920,3840,3840 imgsize=0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 tilewidth=1920 tileheight=1920 ver=807,808,809,810,811,812,813,814,815,816,817,818,819,820,821,822,823,824,825,826,827,828,829,830,831,832,833,834,835,836,837 oldwid=0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0 wid=0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0