Last Comment Bug 710774 - Qt needs to dispatch DID_PAINT events
: Qt needs to dispatch DID_PAINT events
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Widget: Qt (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Oleg Romashin (:romaxa)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-14 10:05 PST by Boris Zbarsky [:bz]
Modified: 2012-03-20 03:55 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Widget Wil/Did Paint (3.81 KB, patch)
2012-03-17 19:05 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Widget Wil/Did Paint (3.79 KB, patch)
2012-03-17 22:47 PDT, Oleg Romashin (:romaxa)
no flags Details | Diff | Review
Widget Wil/Did Paint (3.79 KB, patch)
2012-03-17 22:49 PDT, Oleg Romashin (:romaxa)
roc: review-
Details | Diff | Review
Widget Wil/Did Paint (4.13 KB, patch)
2012-03-18 00:59 PDT, Oleg Romashin (:romaxa)
roc: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2011-12-14 10:05:26 PST
See bug 598482 comment 79.
Comment 1 Oleg Romashin (:romaxa) 2012-03-17 19:05:41 PDT
Created attachment 606923 [details] [diff] [review]
Widget Wil/Did Paint
Comment 2 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-17 20:39:10 PDT
Comment on attachment 606923 [details] [diff] [review]
Widget Wil/Did Paint

Review of attachment 606923 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/qt/nsWindow.cpp
@@ +1061,5 @@
> +    {
> +        nsEventStatus status;
> +        nsPaintEvent willPaintEvent(true, NS_WILL_PAINT, this);
> +        willPaintEvent.willSendDidPaint = true;
> +        DispatchEvent(&willPaintEvent, status);

Is it really safe to change widget geometry here (and where you call DispatchDidPaint)?

@@ +1103,5 @@
>  #endif //MOZ_ENABLE_QTMOBILITY
>  
> +        status = DispatchEvent(&event);
> +        DispatchDidPaint(this);
> +        aPainter->beginNativePainting();

maybe this should be "endNativePainting"?

And maybe that should happen before DispatchDidPaint?
Comment 3 Oleg Romashin (:romaxa) 2012-03-17 22:47:06 PDT
Created attachment 606937 [details] [diff] [review]
Widget Wil/Did Paint

Fixed endNativePainting.

> Is it really safe to change widget geometry here
Did not get why geometry should change here?
> (and where you call DispatchDidPaint)?
I call it at the end of GL painting and at the end of DoPaint (software rendering)
Comment 4 Oleg Romashin (:romaxa) 2012-03-17 22:49:31 PDT
Created attachment 606938 [details] [diff] [review]
Widget Wil/Did Paint

Err, forgot to fix order
Comment 5 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-17 23:32:54 PDT
Comment on attachment 606938 [details] [diff] [review]
Widget Wil/Did Paint

Review of attachment 606938 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/qt/nsWindow.cpp
@@ +1233,5 @@
>      }
>  
>      ctx = nsnull;
>      targetSurface = nsnull;
> +    DispatchDidPaint(this);

You're not setting willSendDidPaint to true for this paint event.
Comment 6 Oleg Romashin (:romaxa) 2012-03-18 00:59:05 PDT
Created attachment 606949 [details] [diff] [review]
Widget Wil/Did Paint

Added WillSendDidPaint flag to software paint.
Also not sure what to do about geometry changed in this case... I guess nothing bad should happen
Is there are test related to this move/size during paint?
Comment 7 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-03-19 02:05:18 PDT
I can't remember. It doesn't happen much anymore since we got rid of almost all child widgets.
Comment 8 Oleg Romashin (:romaxa) 2012-03-19 02:17:10 PDT
Comment on attachment 606949 [details] [diff] [review]
Widget Wil/Did Paint

Ok, hopefully we can survive with that
Comment 10 Mounir Lamouri (:mounir) 2012-03-20 03:55:20 PDT
https://hg.mozilla.org/mozilla-central/rev/d548cc64e028

Note You need to log in before you can comment on or make changes to this bug.