Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zooming via pinch to zoom causes graphical glitches #404

Closed
taaem opened this issue Nov 28, 2018 · 83 comments · Fixed by #1528
Closed

Zooming via pinch to zoom causes graphical glitches #404

taaem opened this issue Nov 28, 2018 · 83 comments · Fixed by #1528
Assignees
Labels

Comments

@taaem
Copy link
Contributor

taaem commented Nov 28, 2018

When zooming via mulittouch pinch to zoom gestures, the page "glitches" it zooms correctly but it also shows the fullscreen view every few moments, which makes zooming really hard.
Also zooming in general causes the view to scroll quite often so even if I'm using the handle in the menu I end up on the last page even if I started in the middle or somewhere.

@andreasb242
Copy link
Contributor

Link to #402 and #403

All bugs are not reproducible on my system (Ubuntu 16.04)

@morrolinux
Copy link
Contributor

I also have the de-centered scrolling issue when zooming via handler buttons. I think it might be somehow fixable by relative-scrolling the page while zooming in/out, to keep it centered. However, last time I tried to do so, the obtained behaviour was irregular and not desired. maybe I did not take into account all the zoom and scale variables.
I could be doing some testing this weekend if I have enough time.

@cass00
Copy link

cass00 commented Nov 29, 2018

Centered pinch to zoom works quite alright for me. Here is the commit from a while ago from my xournalpp fork cass00/xournalpp@29f43991 . Not actually sure by what way it made it into the main repo. So there is certainly room for improvement.

@andreasb242
Copy link
Contributor

@cass00 This is merged. For me this is working if I use the touch and two fingers. (on Ubuntu 16.04)

@markwmuller
Copy link
Contributor

I'm not sure this is the original issue, but my pinch to zoom is also somewhat broken. Video here:
https://imgur.com/a/d3uxU5p

Using Ubuntu 18.04, Wayland, on Thinkpad x1 yoga. (No pinch to zoom using X, I'm assuming that's expected).

@andreasb242
Copy link
Contributor

@markwmuller
No, it is working for me on Ubuntu 16.04 on a Lenovo Helix with X11. So this is also an issue. We had already an issue with Wayland. Chan you check if Zoom is working e.g. in Evince or EOG or so?

ps. Offtopic, I know: I installed Ubuntu 18.04 first on my Helix. But if I have the Wacom Tablet active (pen removed) Ubuntu crashes on the Login screen. Did you also have Problems, or was it working out of the Box?

@markwmuller
Copy link
Contributor

No pinch-to-zoom in EOG or evince, either, using X.

18.04 worked well for me, without issues as far as wacom/touch goes.

@taaem
Copy link
Contributor Author

taaem commented Dec 2, 2018

My zooming looks exactly like in the video, but I have pinch to zoom in evince on wayland working fine.

@andreasb242
Copy link
Contributor

I can not reproduce it on my system, it is correct working. And I have currently no other Touch enabled device for Testing.

The method XournalView::zoom_gesture_scale_changed_cb is called when the zoom changes.

Please add the line:

printf("zoom: %lf\n", scale);
And then we interpret the output.

@markwmuller
Copy link
Contributor

@andreasb242 Where do I add the line of code?

FWIW, I have it working in x too, now. For me, the fix was to use libinput driver, rather than wacom, from here ( https://bugs.launchpad.net/ubuntu/+source/xf86-input-wacom/+bug/1774242 ). Now zoom works well in evince, and in Xournal++

@andreasb242
Copy link
Contributor

At the beginning of XournalView::zoom_gesture_scale_changed_cb

@taaem
Copy link
Contributor Author

taaem commented Dec 3, 2018

Here is a log with that line added. I think it is related to PDFs in some way, because if I just open Xournal in a new window without having changed anything pinch-to-zoom works fine, but if I open a PDF like shown in the log something messes up.
log.txt

@andreasb242
Copy link
Contributor

Ok, thank you, I'll try with a PDF, if can reproduce it. I can see the really big changes in Zoom, so already the Gesture sends really big differences, which causes the big size changes, e.g.

zoom: 1,501744
zoom: 654,690938

This is really a big jump.

@markwmuller
Copy link
Contributor

For me, it happens without opening a PDF (on wayland). Log below:

jitters.txt

@andreasb242
Copy link
Contributor

@markwmuller There is a really little jitter compared to @taaem

@taaem
Copy link
Contributor Author

taaem commented Dec 6, 2018

Okay it seems it only happens with a specific pdf, can I send you a link to the pdf which causes this behavior? @andreasb242

@andreasb242
Copy link
Contributor

@taaem
For sure, you find my Email address in the Git History, I'll check if it also happens for me.
But I cannot promise you a fix time or so, there is really much going on at the Moment, as you see, and I don't have that much time ;-)

@andreasb242
Copy link
Contributor

Xournal++ probably cannot fix this issue, I looked at an example PDF, which is big and loads slow, but except of this was nothing special.

Should we smooth the values mathematically? Add an option to enable this, and hope this makes it better, even this would not be a real solution, it should probably fix the issue.

What do you think
@taaem @markwmuller ?

@taaem
Copy link
Contributor Author

taaem commented Dec 16, 2018

Did you test it with the pdf on wayland? On xorg I don't have pinch to zoom at all.
But smoothing should definitely help. And it is a workaround but one that isn't to hacky I'd say

@andreasb242
Copy link
Contributor

Did you test it with the pdf on wayland?
No, I have currently only one Notebook with running wayland, and it has no Touchscreen.
I won't test this today for sure.

If you whish to help:
please install gtk-3-example and search for an example with Zoom gesture, if you find one, check if it's working.
If it's working we can take this as example for Xournal++.

@taaem
Copy link
Contributor Author

taaem commented Dec 17, 2018

I just checked and it works perfectly smooth in gtk3-demo

@andreasb242
Copy link
Contributor

Ok, so we should compare it with our. And find the difference ;-)

@taaem
Copy link
Contributor Author

taaem commented Dec 17, 2018

Just found something out, the zooming bug only happens if the pdf was already scrolled past the first page. So on the first page zoom works fine but once on page 2 or higher it is broken.

@andreasb242
Copy link
Contributor

I expected something like this, I currently expect that some coordinates are relative to the page instead of absolute of the screen or something like this.

@taaem
Copy link
Contributor Author

taaem commented Dec 17, 2018

Don't know if that's related but in presentation mode I can only zoom in and not out. But the bug doesn't happen.

@peetCreative
Copy link
Contributor

That's similar to the ctrl + scroll zoom problem in ZoomControl.cpp
event->direction is GDK_SCROLL_SMOOTH so it goes to zoom out directly.
I'm working on it

@taaem
Copy link
Contributor Author

taaem commented Feb 8, 2019

Okay also this isn't working in Evince, so there is something completely off. At this point I'm really disappointed with Lenovo or Wacom or whoever managed to get this in this state (eg no useable pen whatsoever).

@andreasb242
Copy link
Contributor

Install gtk3-demo, there is also an input test, where you can test if you have multi touch working etc.

@taaem
Copy link
Contributor Author

taaem commented Feb 8, 2019

Okay also this isn't working in Evince or the gtk3-demo, so I either have no functional pen (weird glitches) and weird zooming on Wayland or I have a working pen and no multitouch gestures on Xorg. At this point I'm really disappointed with Lenovo or Wacom or whoever managed to get this in this state.

@johrpan
Copy link

johrpan commented Feb 11, 2019

I also have this problem (ThinkPad L380 Yoga) and because you mentioned, I tested my input devices with gtk3-demo. I noticed a difference between touchscreen pinch-to-zoom (which doesn't work correctly in Xournal++) and track pad (which works as expected). This colored box stays where it is after the gesture ends when using the track pad and disappears when using the touchscreen. This means, there is some kind of difference between them, but I don't know whether this helps...

@peetCreative peetCreative added the Zoom Every issue connected to zoom label Feb 23, 2019
@jclsn
Copy link

jclsn commented Apr 22, 2019

To warm this up a little bit. I have absolutely no glitches with evince. The zooming behaviour is nice and smooth! I do have problems with zooming in Xournal++ though, no matter whether I use pinch to zoom or Ctrl +/-, it always scrolls and zooms at the same time, which results in the mentioned graphical glitches.

@jclsn
Copy link

jclsn commented Apr 29, 2019

I realized that the bug occurs only in large documents. On the first page there is usually no problem, but on the latter pages (like page 200 or so) the scrolling effect gets worse and worse. Maybe this would be something worth looking into!

@cbm755
Copy link
Contributor

cbm755 commented Apr 29, 2019

I think it tries to redraw several times during the pinch-and-zoom. I've noticed that's its slower/more awkard on pages with complicated figures.

Update: I meant when annotating PDF files.

@Scafir
Copy link

Scafir commented May 15, 2019

I also have this issue. This issue is triggered when as the scrolling bar appears. The scale factor given by the gesture then goes numb ~80% of the time: https://imgur.com/a/76UEdVi.
I tried successfully copying how evince's zoom works in xournalpp, but the problem is still here indicating that it appears when the zooming is applied, so here (src/control/ZoomControl.cpp):

void ZoomControl::fireZoomChanged()
{
        XOJ_CHECK_TYPE(ZoomControl);

        if (this->zoom < this->zoomMin)
        {
                this->zoom = this->zoomMin;
        }

        if (this->zoom > this->zoomMax)
        {
                this->zoom = this->zoomMax;
        }
        for (ZoomListener* z : this->listener)
        {
                z->zoomChanged();
        }
}

One thing worth noticing though, is that the scrolling is buttery smooth and 100% works when the GTK Touch/Scroll workaround is enabled

@taaem
Copy link
Contributor Author

taaem commented Jul 9, 2019

I was noticing, that EOG and Evince are both pure C apps and have no issue with zooming whatsoever. (EOG's zooming is really smooth)
Since Xournalpp uses C++ maybe the issue is somewhere in the bindings and not in Xournalpp or Gtk?

@LittleHuba
Copy link
Member

Is this bug still there? This ticket has evolved to a lot of different topics. Please state a clear description of the problem so we can fix the issue.

@taaem
Copy link
Contributor Author

taaem commented Sep 10, 2019

The bug is still there, description below:

On Wayland pinch to zoom causes the zoom level to "jump" between the real values and wrong ones (can be lower or higher) as seen in @Scafir s video. The outcome is that you can't use pinch to zoom because you might end up zoomed in 700x.
On Evince pinch to zoom works just fine.
For me this only happens on Wayland, on Xorg I can use the pinch t zoom gesture just fine.

@LittleHuba
Copy link
Member

Could you please switch to the new input system in the settings and retry?

@taaem
Copy link
Contributor Author

taaem commented Sep 10, 2019

@LittleHuba just tried and it is not helping :/

@LittleHuba
Copy link
Member

Okay it seems like we need to finally implement the zoom gesture on our own. I thought about implementing it myself for another ticket but we then found a simpler solution for it.

The zoom gesture is all GTK+ internal and fixing it will be harder than just writing down the code for zooming in our own handler.

@taaem
Copy link
Contributor Author

taaem commented Sep 10, 2019

Okay, thanks!
If I can help in some way please let me know!

@LittleHuba
Copy link
Member

Are you fluent in C++? 😉

@taaem
Copy link
Contributor Author

taaem commented Sep 10, 2019

It's a long time since I last used C++ 😄
I don't know if it is enough for this kind of implementation, but if you can give me hints at were to start I can try 🤷‍♂️

@LittleHuba
Copy link
Member

You can at least try!

The file you should implement this is:
src/gui/inputdevices/TouchInputHandler.cpp

In this file you find a function called handleImpl() which is called for each event that is received. You can differenitiate between events of different fingers using the attribute event->sequence. All the other important values are also stored in event.

Currently this class handles scrolling for touchscreens as this is also problematic within GTK+ for some devices...

An outline of the steps you need to do:

  • Disable the current check that only allows single-touch as you will require multi-touch
  • Rewrite the scrolling routine so that it will ignore multi-touch (reuse the current check)
  • Write the zooming algorithm

Implementation details for the zooming algorithm:

  • get the zoom factor by using the distance of the two fingers
  • use the center of the gesture to scroll the view accordingly (you can use parts of the scrolling algorithm in this file for doing this)

For testing purposes you can just disable the GTK zoom handling by unticking the checkbox for the zoom gesture in the settings dialog.

It is no problem if you are unable to do everything. I'll just adopt your work and finish if it should be required.

Happy coding!

@taaem
Copy link
Contributor Author

taaem commented Sep 10, 2019

Thanks for the detailed information! I will look into it in the coming days!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.