dynamic-zoom: add dynamic zoom to impress#15090
Conversation
8514ae0 to
203850b
Compare
203850b to
edfbf64
Compare
|
Thanks for the update:
|
|
I seem to get some grim tiling problems with this - but perhaps without as well:
$ make run @gokaysatir is that a known issue ? |
|
@mmeeks & @gokaysatir i tried to reproduce this and i noticed a significant delay in receiving/rendering new tiles.
|
|
i also noticed flickers when i resize the window, i will look into that first since that is a part of "dynamic zoom" and i might find something related to the above issue in the process, let's see. |
|
so it seems that the invalidation code is running a few times more than it should (ideally once). /* we have this in CanvasTileLayer.js */
this.resObserver = new ResizeObserver(function() {
that._layer._syncTileContainerSize();
});
this.resObserver.observe(canvasContainer);
/* and later down the line we have this */
this._map.on('resize', this._syncTileContainerSize, this);
/* `_syncTileContainerSize` calls `this._map.invalidateSize` which
* fires a 'resize' event at the end, which then again triggers
* `_syncTileContainerSize`. and it also calls `_fitWidthZoom` which
* itself is listening for 'resize' events.
*/ |
I couldn't see the tile rendering issues with the test file but the 3rd slide takes really long time to load. Not sure if that's related to images. |
|
The images are 4k images I think, or bigger but are block colors so they compress small. They stress our scaling code. But we badly need to make sure we are not re-scaling them multiple times - so we should get our zoom level right on the wire message from JS -> browser as early as possible I think. It would be really bad to render once in the backend at one zoom, then zoom and re-render and re-scale the images to another size - incredibly inefficient. |
74572be to
d0bbb25
Compare
242dd71 to
8c09d49
Compare
8c09d49 to
70cacea
Compare
Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: I47947b30be54333fe67c52166ea58d22e0d91ed9
For writer, it was quite simple, we just had width to account for and compare against. But in impress we have to consider both the width and the height of the slide and based on that decide on a zoom level at which the whole slide is fully visible. Initially the idea was to adjust the document dimensions to have 8% to 14% margins in extreme cases and calculate the zoom. But turns out the zoom averages around 100% in that case because as the window size increased, so did the margins (percentage) and the ratio balanced itself. So we settled on a fixed margin. 50px works quite nicely, 25px on each side. Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: Ieb7bb02b707cb3b0351f2fc3badcd7e02077502e
…abel Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: Ice381c5da2eebb33c804fc1eefdd9f7e6310aade
…m reset Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: I09a75357a10b8ecb0df49322c4d67022f6bc5eaf
Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: I67cd1a1d86b22a93eca73afea271d0bf369b890e
Previous patches handled impress similar to writer, but both are quite different. In impress we want the slide to change zoom (size) as we resize the window. Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: I03693e622682fb718d89d4b597806d7e54674b4b
Previously this function set a flag and relied on the 'resize' events to call `_fixWidthZoom` which then checked this flag and recalculated the zoom. But since `_fixWidthZoom` is no longer triggered by 'resize' (as we removed that a few commits back), we need to explicitly call `_fixWidthZoom` here. Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: I5391744b6f1210dd897262935967fc2cff03e826
The slide's zoom isn't static anymore, i.e. if the sidebar shows up, the slide zooms out, therefore the dimensions of the image on the slide will also be different. Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: I6753484048e412e098946923c13734cd6ba0bdd3
Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: Id0eca9f16fe7c06b1c741b96522ce4d969ec2c50
70cacea to
0d1dff4
Compare
Since we removed the multiple resize listeners from the slides a few commits back, zoom changes sometimes (in the tests) don't change the actual zoom of the slide. There should be some time between these clicks and the "has the zoom changed" check. This gives the application enough time to change the document/slide zoom. Signed-off-by: Sahil Gautam <sahil.gautam@collabora.com> Change-Id: If37e782e01611ceba9a2bc6eb6f05e910c2f64e7
|
https://gerrit.collaboraoffice.com/c/online/+/2588 moved to gerrit |




Summary
TODO
Checklist
make prettier-writeand formatted the code.make checkmake runand manually verified that everything looks okay