Incorrect BView::Draw() Update Rect after CopyBits()

[bonefish]

hrev25860

After a sequence of these method invocations:

Invalidate((35.000000, 345.000000) - (41.000000, 359.000000))
Invalidate((0.000000, 345.000000) - (6.000000, 359.000000))
Invalidate((0.000000, 345.000000) - (6.000000, 359.000000))
CopyBits(((0.000000, 15.000000) - (560.000000, 388.000000)) -> (0.000000, 0.000000) - (560.000000, 373.000000))
Invalidate((0.000000, 345.000000) - (559.000000, 359.000000))

the view receives the following Draw() method call:

TermView::Draw((0.000000, 0.000000) - (560.000000, 373.000000))
Bounds(): (0.000000, 0.000000) - (560.000000 - 373.000000)
clipping region:
  (0.000000, 330.000000) - (6.000000, 344.000000)
  (35.000000, 330.000000) - (41.000000, 344.000000)
  (0.000000, 345.000000) - (559.000000, 358.000000)
  (0.000000, 359.000000) - (560.000000, 373.000000)

The update rect covers the whole view, although the clipping region is constrained to 44 pixel rows at the bottom.

[mmlr]

This might be intended behaviour. In hrev25854 the behaviour for calling Draw() has been adjusted to work the same as under R5. Under R5 you will get Draw() calls also for regions that do not intersect with the current clipping, which allows caching and asynchronously repainting of exposed areas as done by Firefox for example. Could you please check if this also happens under BeOS? Note that the clipping region will still limit the drawing to the same area, even though the update rect might be bigger.

[bonefish]

The behavior can be observed in hrev25853 as well. Unfortunately I haven't been able to reproduce the behavior with a minimal test app yet. Might be timing related. It is relatively reliably reproducible with the Terminal in the state I have it ATM.

At any rate I really can't see how supplying an unnecessarily large update rect to Draw() can have any positive effects. Even if the rect is cached and used as a clipping region for later drawing outside Draw() the minimal update rect should suffice. The only effect I can see is that more work than necessary is caused.

[stippi]

The fact that you cannot reproduce this in a test app is interesting. Knowing how the update mechanism works, I would be tempted to say that other views in the view hierarchy are being invalidated as well. But since this is Terminal, it is hard to see which ones. The update rect is unfortunately one and the same rect for all the views that are invalidated in one "session" (the frame of the dirty region). It is clipped to the respective view bounds on the client side. In another words, if there is a view being redrawn to the top of the view you are interested in, you would get an invalidation rect that extends to the top of that view no matter what parts you invalidated. I could change this, so that each view gets it's own update rect. I am looking at the clipping region of each potentially affected view anyways to know whether or not it should be put into the update session. However, the circumstances with the Terminal might hint at another problem with CopyBits(). Have you already taken a look at the update message that arrives at the window? Is the TermView the only one being redrawn? BTW, I can see what you are trying to do and why this behavior is definitely a problem.

[bonefish]

You are right, another view is invalidated in this case. When hitting enter in the last line of the terminal screen the history size changes and thus the scroll bar range is updated. Simulating this situation in the test app, I can reproduce the same update rect behavior there, too.

ATM I'm working around in TermView::Draw() by getting the frame of the clipping region, but this is of course more expensive than doing it in the app server in the first place.

Under BeOS the test app gets the correct (i.e. minimal) update rect, BTW.

[stippi]

Ah, great, so it isn't something mysterious. Yeah, I guess I can fix this for you.

[stippi]

Could you please check if hrev25879 fixed this issue for you?

[bonefish]

Confirmed fixed. Thanks!