Closed Bug 551431 Opened 14 years ago Closed 14 years ago

WidgetQt Show/Resize functionality need to be optimized

Categories

(Core Graveyard :: Widget: Qt, defect)

Other
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jaakko.kiviluoto, Unassigned)

Details

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.2pre) Gecko/20100309 Ubuntu/10.04 (lucid) Namoroka/3.6.2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.3a3pre) Gecko/20100303 Namoroka/ Fennec/1.1a2pre

Currently show/resize functionality of the Qt port is not optimized at all, and we have a lot of additional on/resize events which are making browser startup/show slower.

Reproducible: Always
Attached patch Proposed patchSplinter Review
Attachment #431612 - Flags: review?
moving to correct component
Status: UNCONFIRMED → NEW
Component: Linux/Maemo → Widget: Qt
Ever confirmed: true
Product: Fennec → Core
QA Contact: maemo-linux → qt
Comment on attachment 431612 [details] [diff] [review]
Proposed patch

Are we losing an optimization (or a required update) by removing lines like this:

-    if (aRepaint)
-        mWidget->update();
Attachment #431612 - Flags: review? → review?(dougt)
Comment on attachment 431612 [details] [diff] [review]
Proposed patch

when are:
+    PRPackedBool mListenForResizes;
+    PRPackedBool mNeedsShow;

set to true?


+        // Are the bounds sane?
+        // Yep?  Resize the window


No check, so how do you know if they are sane?

mark as concerned about:

-    if (aRepaint)
-        mWidget->update();

Please address his comment.
Attachment #431612 - Flags: review?(dougt) → review-
Oleg, can you address these, please?
> -    if (aRepaint)
> -        mWidget->update();
> 
> Please address his comment.

Originally I was trying to merge Gtk to Qt and all related fixes...

and in Gtk port we are calling gdk_window_move_resize, and no separate invalidate call and aRepaint not used...

Also force repainting is bad idea, and not possible in QGV. Qt should handle it inside, and request update if it is needed...
Attachment #431612 - Flags: review- → review+
Attached patch Updated patchSplinter Review
(In reply to comment #4)
> (From update of attachment 431612 [details] [diff] [review])
> when are:
> +    PRPackedBool mListenForResizes;
> +    PRPackedBool mNeedsShow;
> 
> set to true?

Now done as in gtk2 nsWindow.

> +        // Are the bounds sane?
> +        // Yep?  Resize the window
> 
> No check, so how do you know if they are sane?

Bound check now done for resizes.

> mark as concerned about:
> 
> -    if (aRepaint)
> -        mWidget->update();
> 
> Please address his comment.

Restored these, since aRepaint is mostly PR_FALSE anyway, so these didn't really improve the prformance.

Tested with Fennec/1.1a2pre + xulrunner 1.9.3a4pre (20100317-1.9.3-xr2). No regressions observed.
Attachment #433322 - Flags: review?(dougt)
Attachment #433322 - Flags: review?(dougt) → review?(romaxa)
Comment on attachment 433322 [details] [diff] [review]
Updated patch

>diff -U8 -r -p mozilla.orig/widget/src/qt/nsWindow.cpp mozilla/widget/src/qt/nsWindow.cpp
>--- mozilla.orig/widget/src/qt/nsWindow.cpp	2010-03-18 12:29:52.000000000 +0200
>+++ mozilla/widget/src/qt/nsWindow.cpp	2010-03-18 14:58:37.000000000 +0200
>@@ -181,16 +181,20 @@ nsWindow::nsWindow()
>     mWidget              = nsnull;
>     mIsVisible           = PR_FALSE;
>     mActivatePending     = PR_FALSE;
>     mWindowType          = eWindowType_child;
>     mSizeState           = nsSizeMode_Normal;
>     mPluginType          = PluginType_NONE;
>     mQCursor             = Qt::ArrowCursor;

>     mNativeWidget        = nsnull;
This is not yet part of trunk, small fixreject required for this patch to be applied on m-c



>+    
Also some white spaces here and there


But in general it looks good
Attachment #433322 - Flags: review?(romaxa) → review+
Pushed:
http://hg.mozilla.org/mozilla-central/rev/283eb9a9cbf1
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: