227733 - Live resize is possible for BeOS widget implementation.

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[Sergei Dolgov]

Some times ago we decided to create native windows with B_OUTLINE_RESIZE flag due 
overhead of resize events generated.
One of reason of that event's flood was existence of OnResize() call in Resize()
method.
So previous porting team create tricky workaround with ResizeRunner. Which
didn't help well, though.

Now i tried straight logical way, like it was done in MSWindow port.
added B_FRAME_EVENTS flag to both nsWindowsBeOS and nsView BeOS,
implemented nsViewBeOS::FrameResized() and rewrote 
nsWindowBeOS::FrameResized().

So both those now calls async method with ONRESIZE tag.
Removed B_OUTLINE_RESIZE flag, removed ResizeRunner and DoFrameResize.

Also removed OnResize() from Resize() method.

All seems working quite nice. Resize with content seems even more smooth than in
NetPositive.

Contunue testing for unwanted effects


It seems that

Comment 1

[Sergei Dolgov]

adding Paul

Comment 2

[Sergei Dolgov]

Attachment: Patch (diff -up)

Removes old ugly hack to process resize events.
Now BView resize events are processed exactly as all other events.
Thus removed lot of obsolete and unneccessary methods.

Additionaly removed some hacks for BWindow activation events (focus handling).
It seems we don't need those anymore.

Some identation fixed (probably was broken in process of applying -w patch.

For testing and estimation purpose B_OUTLINE_FLAG is commented out, so we can
see how god or bad in speed that resizing is

Comment 3

[Sergei Dolgov]

Attachment: Patch (diff -up)

as users are glad with "live-resize" instead B_OUTLINE, submitting final patch

Comment 4

[Sergei Dolgov]

Comment on attachment 139272 [details] [diff] [review]
Patch (diff -up)

review request.
Explanation:
All native UI events, hooked by BView and BWindow methods were processed in
same way.
Hook methods got nsWindget and nsToolkit and then invoked nsToolkit method
CallMethodAsync (thread-proxy, as each BWindow is separate thread).
Those calls via messages pass to main UI thread method ::CallMethod() which
dispatches then nsEvents.

Only exclusion was case of Resize events.

THis patch unifies BeOS-native resize events' pass-through with all others.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 139272 [details] [diff] [review]
Patch (diff -up)

+			uint32 flags = B_ASYNCHRONOUS_CONTROLS /*|
B_OUTLINE_RESIZE*/ | B_FRAME_EVENTS;

why did you comment this out, instead of just removing it?

+			else
+				OnMove(aX,aY);
hm... why do you need to handle a NULL BView/BWindow now, but not before?

+//	printf("Resize\n");
+//	fflush(stdout);
please remove this

+				((nsWindowBeOS
*)mView->Window())->ResizeTo/*WithoutEvent*/(aWidth-1, GetHeight(aHeight)-1);

is there a reason to leave the commented out /*WithoutEvent*/ in there?

+				OnResize(mBounds);;
one semicolon too much...

+		//if(mJustGotActivate) 
why did you comment this out?

+				r.x = (nscoord)br.left; 

please declare the nsRect here, like this:
nsRect r(nscoord(br.left), nscoord(br.top), ...);
also, why do you add 1 to the return value of IntegerWidth()?

 void nsWindowBeOS::MessageReceived(BMessage *msg)
this message only calls BWIndow::MessageReceived now. why do you override it at
all?

+//	printf("VFrameResized\n");
please remove

+	if (width==lastViewWidth && height==lastViewHeight)
please put {} around the block after this if

+//	printf("VFrameMoved\n");
again, please remove

also - can you explain whyyou moved FrameResized to nsViewBeOS from
nsWindowBeOS, and why you did not move FrameMoved (but implemented it in both)?


+	lastWindowPoint.x = origin.x;
can you not just do lastWindowPoint = origin; ?

Comment 6

[Sergei Dolgov]

>B_OUTLINE_RESIZE*/ | B_FRAME_EVENTS;
>why did you comment this out, instead of just removing it?
maybe in future i will implement various resize mode for various window types,
so that's reminder. Maybe is better to put that comment after end of statement.

>hm... why do you need to handle a NULL BView/BWindow now, but not before?
True. It is handled before. Will remove. As extra semicolon.

>*)mView->Window())->ResizeTo/*WithoutEvent*/(aWidth-1, GetHeight(aHeight)-1);

>is there a reason to leave the commented out /*WithoutEvent*/ in there?
will change comment to 
//XXX:Here was call to ResizeWithoutEvents, will look if we need something like it. 

Also check for 0 for BWindow to remove. as in previous comment.

//if(mJustGotActivate) 
why did you comment this out? this is strange hack borrowed from MS Win code.
For now it seems working without this hack, but who knows. So placeholder. As
all other outcommented things in activation/focus path. As we haven't special
testers here in BeOS - let users test it from regular builds

>+				r.x = (nscoord)br.left; 
>
>please declare the nsRect here, like this:
>nsRect r(nscoord(br.left), nscoord(br.top), ...);
There was loop previously here, so i was afraid that recreating nsRect in stack
 in constructor is slower, than declaring it outside loop and assign values to
members inside loop. For current situation it don't matter, so will do
>also, why do you add 1 to the return value of IntegerWidth()?
Heh, you ask:( This is main origin of my headache in mozdevelopment. Mozilla and
Be API have different imagination about rects (exclusive and inclusive).
So we adding everywhere 1 if convert BeOS with to mozilla width, and vice versa.
Especial headache it provides in nsImage, but that's different story.
> void nsWindowBeOS::MessageReceived(BMessage *msg)
>this message only calls BWIndow::MessageReceived now. why do you override it
>at all?
I have big plans for it. Especially for DND, which is yet implemented in BeOS
port. Also we have some problems with destroy processes, and this is essential
place to refine quitting in Be API.

>also - can you explain whyyou moved FrameResized to nsViewBeOS from
>nsWindowBeOS, and why you did not move FrameMoved (but implemented it in >both)?

Well. We don't have separate nsWindow and ns Widget, as gtk does. Unfortunately.
Maybe if i have month of free time, i will rewrite it.
At moment this code was written by MS Window model, but actually,

nsWidget is BView. So all such events are related to mView, Window is only
container. Also, each BWindow frame event causes similar events for its
children, and above in BView/nsViewBeOS constructor i have put flags
B_FOLLOW_ALL and B_FRAME EVENTS. It means that BView really gets notifications
about container BWindow events. 
Most important reason for slowness and hacks in previous implementation was just
idea about putting BWindow::FrameResized in game. So removed.

For FrameMoved i put explanation in comment. We need it to remember Mozilla
windows position after quit. BView::Frame resized don't get notifcation about
moves of whole window in screen coordinates.

All other comments about {} and debug printfs will be taken in account:)

Comment 7

[Sergei Dolgov]

Also additional comment for your education about FrameMoved.
In other platforms notification about container window move may be much more
important - to allow or disallow content update while dragging window over screen.
In BeOS we don't need it. Window content update while dragging is care of
ApplicationServer, and it does it supereffectively. Be API even hasn't mode/flag
for "outline move". And don't produce repaint notification ins such process.
AFAIK, "Live moving" is(was) bit trouble for MS Win. But not for BeOS

Comment 8

[Sergei Dolgov]

Attachment: Patch (diff -up against HEAD)

Notices taken in account.

Comment 9

[Sergei Dolgov]

Comment on attachment 139356 [details] [diff] [review]
Patch (diff -up against HEAD)

next attempt for review

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

Checking in nsWindow.cpp;
/cvsroot/mozilla/widget/src/beos/nsWindow.cpp,v  <--  nsWindow.cpp
new revision: 1.75; previous revision: 1.74
done
Checking in nsWindow.h;
/cvsroot/mozilla/widget/src/beos/nsWindow.h,v  <--  nsWindow.h
new revision: 1.30; previous revision: 1.29
done

Comment 11

[Sergei Dolgov]

Actually i meant such algorythm. Prefilling first mHeight rows in target bitmap
in simple 2d loop, then memcpy in another Y-only loop whole already filled rows.

But cannot predict performance difference at moment.

Only known truth about optimization is that existance of processor cache makes
sometimes loop with single writes much less effective than using big chunks to
write.

Comment 12

[Sergei Dolgov]

Previous comment was for bug 231202.