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

State of build of webkit with QT 6.5.3 on both Linux and Windux (MINGW) #35

Open
4 of 5 tasks
quarcko opened this issue Feb 23, 2024 · 15 comments
Open
4 of 5 tasks

Comments

@quarcko
Copy link

quarcko commented Feb 23, 2024

So, i made it! Webkit with mingw is kinda-working and semi-usable. This is not an issue report but place for discussion:

Issues present in current builds:

  • 1) Scrolling with mouse wheel does not work on both Linux and Windows (mingw)

  • will dive in later, probably related to some of my fixes i did incorrectly in platform events.

  • 2) if accelerated rendering is enabled in QtTestBrowser menu - webkit crashes (Linux and Windows)

  • not really interested in fixing, as i won't use accelerated rendering anyway

  • 3) Some web-pages that use custom fonts (probably) - text is not displayed (windows mingw only problem)

    • maybe some incorrect or missing font rendering libraries? i used MSYS-libs for compilation
  • 4) Webkit hangs during page loads with single warning message (Windows mingw only):

    • QApplication::exec() - must be started from main thread - or something like that.
      i pinpointed that it happens in WTF/wtf/qt/RunLoopQt.cpp function void RunLoop::run()
      on line "QCoreApplication::exec(); possible workaround is to set mainEventLoopIsRunning = true
      so it never does that part:
void RunLoop::run()
{
    static bool mainEventLoopIsRunning = true;
    if (!mainEventLoopIsRunning) {
        mainEventLoopIsRunning = true;
        QCoreApplication::exec();
        mainEventLoopIsRunning = false;
    } else {
        QEventLoop eventLoop;
        QEventLoop* previousEventLoop = currentEventLoop;
        currentEventLoop = &eventLoop;

        eventLoop.exec();

        currentEventLoop = previousEventLoop;
    }
}

but this seems to be not "real" fix, as now there is no freeze, pages load all good, but after some time i got SIGSEGV:

Thread 1 received signal SIGSEGV, Segmentation fault.
0x00007ff9f10e3155 in WTF::RunLoop::dispatch(WTF::Function<void ()>&&) () from C:\Qt\webkit\build-win\bin\Qt6WebKit.dll
  • 5) UPDATE: Huge memory leak discovered (both Linux and Windows)

    • Just try to repeatedly refresh some webpage - and memory uncontrollably goes up.
    • UPDATE: if you leave it rest for some time - memory gets eventually cleared, but it takes too long.
    • If someone knows how this garbage collector is working, we need to call it more often?

so yea, that's all resume for now.

@aquiles2k
Copy link

Thanks for all your work on this!

I can compile the mingw q5 version through mxe without JIT, which is not very usable. Can you push your changes you did to make it compile with JIT enabled? I'm curious how that version will behave.

@quarcko
Copy link
Author

quarcko commented Feb 23, 2024

No problem, i just badly need webkit myself :) without it moving our apps to Qt6 is impossible.

I just push'ed all my changes. please look up in my fork. Branch qt6-gha.

Really interested in your build, especially regarding font rendering and freeze on RunLoop.
Please try to run it with both "workarounded" RunLoopQt.cpp and original.

@mnutt
Copy link
Member

mnutt commented Feb 28, 2024

Regarding scrolling, that currently doesn't work on QtWebKit on any platform, unfortunately. It's specifically a mousewheel problem, as arrow scrolling works ok. IIRC it was something relating to smooth scrolling? Seemed fixable but a nontrivial amount of work.

@elahav
Copy link

elahav commented Feb 28, 2024

Doesn't work on QNX or Linux with Qt 6.6.1 and latest WebCore either, for what it's worth.

@aquiles2k
Copy link

@mnutt Do you have any links for the scrolling/mousewheel problem? Discussions or bugreports?

@elahav
Copy link

elahav commented Feb 29, 2024

It's a real pain to debug due to the huge libraries - every symbol lookup, every single-step takes forever. So far I can tell that the wheel event is noticed by QWebView, and seems to be propagated all the way down to the WebCore wheel event handler:

#0  WebCore::EventHandler::handleWheelEventInternal(WebCore::PlatformWheelEvent const&, WTF::OptionSet<WebCore::WheelEventProcessingSteps>, WTF::OptionSet<WebCore::EventHandling>&) (this=0x7fffbe0c8e00, event=..., processingSteps=..., handling=...)
    at /home/elahav/src/other/webkit/WebKit/Source/WebCore/page/EventHandler.cpp:3117
#1  0x00007ffff4161649 in WebCore::EventHandler::handleWheelEvent(WebCore::PlatformWheelEvent const&, WTF::OptionSet<WebCore::WheelEventProcessingSteps>) (this=<optimized out>, wheelEvent=..., processingSteps=..., processingSteps@entry=...)
    at /home/elahav/src/other/webkit/WebKit/Source/WebCore/page/EventHandler.cpp:3110
#2  0x00007ffff303fe49 in QWebPageAdapter::wheelEvent(QWheelEvent*, int)
    (this=this@entry=0x555555983200, ev=ev@entry=0x7fffffffdbb0, wheelScrollLines=3)
    at /home/elahav/src/other/webkit/linux-x86_64-qt/WTF/Headers/wtf/OptionSet.h:93
#3  0x00007ffff7f9a9c8 in QWebPage::event(QEvent*) (this=<optimized out>, ev=0x7fffffffdbb0)
    at /home/elahav/src/other/webkit/WebKit/Source/WebKitLegacy/qt/WidgetApi/qwebpage.cpp:2669
#4  0x00007ffff7f9e104 in QWebView::wheelEvent(QWheelEvent*) (this=<optimized out>, ev=0x7fffffffdbb0)
    at /home/elahav/src/other/webkit/WebKit/Source/WebKitLegacy/qt/WidgetApi/qwebview.cpp:920
#5  0x00007ffff7a02c3e in QWidget::event(QEvent*) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Widgets.so.6
#6  0x00007ffff79aefb2 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Widgets.so.6
#7  0x00007ffff79b8d1c in QApplication::notify(QObject*, QEvent*) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Widgets.so.6
#8  0x00007ffff16d767a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Core.so.6
#9  0x00007ffff7a164de in  () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Widgets.so.6
#10 0x00007ffff7a17a89 in  () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Widgets.so.6
#11 0x00007ffff79aefb2 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Widgets.so.6
#12 0x00007ffff16d767a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Core.so.6
#13 0x00007ffff1fa99ca in QGuiApplicationPrivate::processWheelEvent(QWindowSystemInterfacePrivate::WheelEvent*) ()
    at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Gui.so.6
#14 0x00007ffff2004b7c in QWindowSystemInterface::sendWindowSystemEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Gui.so.6
#15 0x00007fffe9e595aa in  () at /home/elahav/Qt/6.6.1/gcc_64/plugins/platforms/../../lib/libQt6XcbQpa.so.6
#16 0x00007ffff0097d3b in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#17 0x00007ffff00ed258 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#18 0x00007ffff00953e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#19 0x00007ffff197ef4a in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
    at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Core.so.6
#20 0x00007ffff16e2fc3 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Core.so.6
#21 0x00007ffff16dfa8e in QCoreApplication::exec() () at /home/elahav/Qt/6.6.1/gcc_64/lib/libQt6Core.so.6
#22 0x000055555556d2e5 in launcherMain(QApplication const&) (app=...)
    at /home/elahav/src/other/webkit/WebKit/Tools/QtTestBrowser/qttestbrowser.cpp:64
#23 main(int, char**) (argc=<optimized out>, argv=<optimized out>) at /home/elahav/src/other/webkit/WebKit/Tools/QtTestBrowser/qttestbrowser.cpp:330

@quarcko
Copy link
Author

quarcko commented Feb 29, 2024

Hi everybody,

It's nice to see that you guys also have interest in this abandonware 😆
so few more news from me:

Thanks to support from @annulen from qtwebkit repo i managed to fix issue "4)"
outlined in this post.
I will push the fix to my repo once i do all the tidy-ing up and last check that everything is ok.
So now we have much less crashing, freezing qtwebkit :)

Now i have to take a few days break from webkit to do other responsibilities at work
so if some of you will collect some info regarding "scrolling" (thanks @elahav )
or could try to test on Windows machine if there are missing fonts on some webpages
would be great!

P.S. Also there is huge new issue found recently (multiplatform): memory leak.
Try to repeatedly refresh some light (google.com) webpage -> your webkit memory usage will amuse you.

@mnutt
Copy link
Member

mnutt commented Feb 29, 2024

Unfortunately the WebKit slack channel is limited to 90 days of chat history so I don't have the exact conversation 😢

The gist was that some upstream implementation changed to the point where our wheelEvents aren't even firing scrolling events anymore. IIRC it was maybe something relating to ASYNC_SCROLLING we don't (and probably won't in the near-term) support? The fix likely involves writing a bit of implementation.

Perhaps attaching a debugger to

bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
would be a starting point.

@elahav
Copy link

elahav commented Feb 29, 2024

It gets to the point of starting animated scrolling.

(Apologies for the scattered comments, I'm debugging this during short breaks from Real Work (TM))

@elahav
Copy link

elahav commented Feb 29, 2024

Mouse wheel works if I disable the animation. Tested with a very crude change:

diff --git a/Source/WebCore/platform/ScrollingEffectsController.cpp b/Source/WebCore/platform/ScrollingEffectsController.cpp
index d2858e961caa..0b45ae340a0e 100644
--- a/Source/WebCore/platform/ScrollingEffectsController.cpp
+++ b/Source/WebCore/platform/ScrollingEffectsController.cpp
@@ -421,7 +421,7 @@ bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& whee
     if (!deltaX && !deltaY)
         return false;
 
-#if ENABLE(SMOOTH_SCROLLING)
+#if 0// ENABLE(SMOOTH_SCROLLING)
     if (m_client.scrollAnimationEnabled() && !m_inScrollGesture) {
         if (!retargetAnimatedScrollBy({ deltaX, deltaY }))
             startAnimatedScrollToDestination(scrollOffset, scrollOffset + FloatSize { deltaX, deltaY });

Unless someone wants to dig into the animation code, the solution is likely yo create a delegate that returns false for scrollAnimationEnabled(). Personally I don't see the need for animation - scrolling seems quite smooth without it (based on 1.3 seconds worth of testing...).

@mnutt
Copy link
Member

mnutt commented Feb 29, 2024

I'm good with disabling the animation for now. 👍

@elahav
Copy link

elahav commented Feb 29, 2024

Temporary fix:

diff --git a/Source/WebCore/platform/ScrollAnimator.cpp b/Source/WebCore/platform/ScrollAnimator.cpp                                                  
index 21f1fb375a53..d0e9975cdd45 100644                                                                                                               
--- a/Source/WebCore/platform/ScrollAnimator.cpp                                                                                                      
+++ b/Source/WebCore/platform/ScrollAnimator.cpp
@@ -407,6 +407,14 @@ bool ScrollAnimator::scrollAnimationEnabled() const   
 {   
     return m_scrollableArea.scrollAnimatorEnabled();                      
 }                                                                         
+#elif PLATFORM(QT)                                                        
+bool ScrollAnimator::scrollAnimationEnabled() const                       
+{                                                                                                                                                    
+    // FIXME:
+    // Scrolling animation doesn't work right now.                        
+    // Return false so that non-animated wheel scrolling works.           
+    return false;
+}
 #endif                                                                    
  
 void ScrollAnimator::cancelAnimations()                                   
diff --git a/Source/WebCore/platform/ScrollAnimator.h b/Source/WebCore/platform/ScrollAnimator.h                                                      
index 3aba2efe74d7..ac934eabedfa 100644                                    
--- a/Source/WebCore/platform/ScrollAnimator.h       
+++ b/Source/WebCore/platform/ScrollAnimator.h                             
@@ -163,7 +163,7 @@ private:
     void deferWheelEventTestCompletionForReason(WheelEventTestMonitor::ScrollableAreaIdentifier, WheelEventTestMonitor::DeferReason) const final;    
     void removeWheelEventTestCompletionDeferralForReason(WheelEventTestMonitor::ScrollableAreaIdentifier, WheelEventTestMonitor::DeferReason) const final;         
                                                                           
-#if PLATFORM(GTK) || USE(NICOSIA)                                         
+#if PLATFORM(GTK) || PLATFORM(QT) || USE(NICOSIA)                         
     bool scrollAnimationEnabled() const final;                            
 #endif
  

@quarcko
Copy link
Author

quarcko commented Mar 1, 2024

Thank you @elahav for scrolling fix :) will try it some time later. Which of your diffs should i use, both?

Some update from me:

  1. Fixed font rendering issues on Windows and also much improved on Linux
  2. Fixed memory leak (that i myself introduced with HarfBuzz rendering previous fix)

Everything is up in my fork if someone needs it! :)

@elahav
Copy link

elahav commented Mar 1, 2024

Only the second. You can pick up this commit:

elahav/WebKit@8f9ae93

(I have Emacs remove trailing spaces automatically, which may not be ideal when working on shared projects)

We should probably converge on a single repository. The only reason I created a fork from WebKit is that I believe that this repo does not trail the upstream main branch. If that's correct, it makes it harder to update, or eventually push the changes upstream.

@quarcko
Copy link
Author

quarcko commented Mar 1, 2024

Thank you, just added this one to my build, and scrolling is there! :) perfect.
So apart it might be officially called early alpha :)

Sure, we can collaborate on a single repo, but here you lead the way - i'm not a big expert of git and all that stuff
and don't know how's thats done, except for forking 😆

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

No branches or pull requests

4 participants