sailfishos / sailfish-office

Sailfish Office
GNU General Public License v2.0
73 stars 26 forks source link

[office] Get rid of split view. Fixes JB#41071 #125

Closed jpetrell closed 5 years ago

jpetrell commented 5 years ago

Still WIP

jpetrell commented 5 years ago

I think the pull requests are now ready for review. Got another green light from design on the latest version.

Dropped the use of zoom proxy for text and spreadsheet documents, it is only marginally better performance-wise and comes with many issues of its own. The performance is painfully slow, but IMO would require major rewrite and out of scope of split view pattern removal task.

Presentation performance is not good either (also due to broken architecture), but managed to do few improvements there. First of all postponed loading of next slide until the scroll movement has ended to avoid hiccups in animation. Also pinch zooming is smooth if we avoid texture creation on every frame: https://git.merproject.org/mer-core/calligra/merge_requests/5

Tried to avoid use of legacy ViewController for spreadsheets (and use similar approach to presentation page), but the ContentsModel wasn't very good at rendering spreadsheets in correct dimensions for ImageDataItem so gave up.

jpetrell commented 5 years ago

Fixed few review findings by Pekka:

jpetrell commented 5 years ago

Reinstated the regressed, old cover code back. The cover rendering was really flaky, made image grab async (16ms timer) and postponed the grab until Calligra reports the document status as loaded. It should be now better, but in general am tempted to drop the document previews from the covers due to overall unreliability of having timing-based image grabs/captures.

pvuorela commented 5 years ago

Yea the cover state is a bit sad. I do like how it looks and mostly it works despite the flakiness. Then again we shouldn't have code depending on luck. Ideally we should get the content there with some other method than image grab.

For another case notice there's still something wrong on text document page numbering. On content page +1 item is highlighted compared and tapping a page item takes to either page or page-1.

pvuorela commented 5 years ago

For minor detail on cover, pdf document now includes the page header and pulldown menu content. Guess I can live with that, though.

jpetrell commented 5 years ago

PDF active cover no longer includes pulley or page header, but uses the canvas as the source item. Fixed one offset problem with text document page numbering.

pvuorela commented 5 years ago

Page numbering looks good, but pdf cover also has these properties "for cover state" that didn't seem moved here. Pdf cover itself seemed kind of stretched and having some empty areas.

pvuorela commented 5 years ago

Ok, see pdf reverting a bit to previous state.

LGTM, can fix the cover details separately.