The biggest problem here is that children views do not scroll in sync with the parent, which causes streaks of garbage pixels while scrolling (which eventually get repainted). You can see this on any page which has iframes. The solution will needs to keep them in sync while retaining the hardware screen-to-screen blit for performance. Early experiments have not been that promising - it's a tough problem.
I tried to put existing NS_METHOD nsWindow::Scroll() code with all those blits into children iteration loop, directly replaced in loop mView with its children. All seem stay in sync in this case, but scrolls very slowly.
I'm nor web-design guru, neither mozilla code specialist, but i wonder if BViews' nesting degree in Mozilla rendered page may be more than 1. In such case even this "buggy" scrolling may appear non-working.
confirmed
Code in mozilla/view/* "moves" main view (and children) without repainting by calling SetPosition(). Then checks if widget can BitBlt but not for children. If cannot BitBlt, it invalidates main view and "adjusts" children by itself. BeOS case is a bit messy - coz it gets CanBitBlt property, it don't adjust children, so we should move already "moved" child views. Last fact can be checked if comment out children moving in BeOS widget Scroll () code, scroll some page with inline frames and then resize window - all "unscrolled" children jump on proper position.
By separating the Scroll methods into ScrollWidgets and ScrollRect, we should be able to fix the scrolling of child views. We could, theorectially, lock the looper on the parent, scroll the children, then self, and then, unlock the looper. Currently the BaseWidget contains an empty implementation for the ScrollWidgets and ScrollRect. If am not sure if they are actually being called, yet.
I tried to create both, but those seem never called.
btw, i think that origin of problem is that child widgets are also BeOS BView children in current implementation. Not sure if we really need that, maybe it is possible to add those to parent's BWindow() as children. And Mozilla should care about other aspects of parent/child/sibling relations itself.
Right Sergei. Since they are child BViews (which may or may not be necessary, I don't know) they scroll in the BeOS fashion, which is to change the topleft coordinate of the parent. So when scrolling down 10 pixels, the parent corner is 0, 10 and all the children stay where they are. The Mozilla model as I understand it is to keep the parent corner always at 0, 0 and then move the chidren. Therefore we can't just use ScrollTo or ScrollBy in BView (too bad, because they're hardware accelerated). Right now, you see garbage because CopyBits for the parent is very fast, but moving and redrawing all the children is quite slow (it lags behind). Unfortunately there is no way to tell CopyBits to move the childrens' data also (that's what ScrollTo is for). So other than doing scrolling entirely in software (I tried, it's unusably slow) there are two paths I can think of: 1. Use ScrollTo and ScrollBy but then compensate for the offset (see SetOrigin() and Origin() in BView.h) whenever drawing or dealing with mouse coordinates. 2. Try to implement IFRAMES and whatever else uses child BViews right now in a different way where you have one big view, and then use CopyBits for the scrolling. I'm no longer coding on Mozilla, so feel free to reassign the bug. But I'm happy to discuss ways of fixing this and maybe reviewing the code.
reminder for myself - should look at http://www.3rd-evolution.de/BeOS/MDI/
Daniel, i found wasy how to do bit copy for all page content, only (but seems serious) problem are child ghosts which coexist with scrolled children images. Here is code (maybe not safe yet): if(mView && mView->LockLooper()) { BRect src; BRect b = mView->Bounds(); if(aClipRect) { src.left = aClipRect->x; src.top = aClipRect->y; src.right = aClipRect->XMost() - 1; src.bottom = aClipRect->YMost() - 1; } else src = b; BRegion invalid; invalid.Include(src); // make sure we only reference visible bits // so we don't trigger a BView invalidate if(src.left + aDx < 0) src.left = -aDx; if(src.right + aDx > b.right) src.right = b.right - aDx; if(src.top + aDy < 0) src.top = -aDy; if(src.bottom + aDy > b.bottom) src.bottom = b.bottom - aDy; BRect parentsrc = mView->ConvertToParent(src); BRect dest = parentsrc.OffsetByCopy(aDx, aDy); mView->Parent()->ConstrainClippingRegion(0); if(parentsrc.IsValid() && dest.IsValid()) mView->Parent()->CopyBits(src, dest); invalid.Exclude(mView->ConvertToParent(child->Frame())); invalid.Include(dest); int32 rects = invalid.CountRects(); for(int32 i = 0; i < rects; i++) { BRect curr = invalid.RectAt(i); nsRect r; r.x = (nscoord)curr.left; r.y = (nscoord)curr.top; r.width = (nscoord)curr.Width() + 1; r.height = (nscoord)curr.Height() + 1; OnPaint(r); } mView->UnlockLooper(); } Main idea here is performing copybits operation for Parent() if scrolling-view instead on scrolling-view itself. Those iFrames children appear on corect place and work as links in correct place too. Hope you have time to test it, i applied it together with my last patch for "disappearing images" bug.Daniel, i found wasy how to do bit copy for all page content, only (but seems serious) problem are child ghosts which coexist with scrolled children images. Here is code (maybe not safe yet): if(mView && mView->LockLooper()) { BRect src; BRect b = mView->Bounds(); if(aClipRect) { src.left = aClipRect->x; src.top = aClipRect->y; src.right = aClipRect->XMost() - 1; src.bottom = aClipRect->YMost() - 1; } else src = b; BRegion invalid; invalid.Include(src); // make sure we only reference visible bits // so we don't trigger a BView invalidate if(src.left + aDx < 0) src.left = -aDx; if(src.right + aDx > b.right) src.right = b.right - aDx; if(src.top + aDy < 0) src.top = -aDy; if(src.bottom + aDy > b.bottom) src.bottom = b.bottom - aDy; BRect parentsrc = mView->ConvertToParent(src); BRect dest = parentsrc.OffsetByCopy(aDx, aDy); mView->Parent()->ConstrainClippingRegion(0); if(parentsrc.IsValid() && dest.IsValid()) mView->Parent()->CopyBits(src, dest); invalid.Exclude(mView->ConvertToParent(child->Frame())); invalid.Include(dest); int32 rects = invalid.CountRects(); for(int32 i = 0; i < rects; i++) { BRect curr = invalid.RectAt(i); nsRect r; r.x = (nscoord)curr.left; r.y = (nscoord)curr.top; r.width = (nscoord)curr.Width() + 1; r.height = (nscoord)curr.Height() + 1; OnPaint(r); } mView->UnlockLooper(); } Main idea here is performing copybits operation for Parent() if scrolling-view instead on scrolling-view itself. Those iFrames children appear on corect place and work as links in correct place too. Hope you have time to test it, i applied it together with my last patch for "disappearing images" bug.Daniel, i found wasy how to do bit copy for all page content, only (but seems serious) problem are child ghosts which coexist with scrolled children images. Here is code (maybe not safe yet): if(mView && mView->LockLooper()) { BRect src; BRect b = mView->Bounds(); if(aClipRect) { src.left = aClipRect->x; src.top = aClipRect->y; src.right = aClipRect->XMost() - 1; src.bottom = aClipRect->YMost() - 1; } else src = b; BRegion invalid; invalid.Include(src); // make sure we only reference visible bits // so we don't trigger a BView invalidate if(src.left + aDx < 0) src.left = -aDx; if(src.right + aDx > b.right) src.right = b.right - aDx; if(src.top + aDy < 0) src.top = -aDy; if(src.bottom + aDy > b.bottom) src.bottom = b.bottom - aDy; BRect parentsrc = mView->ConvertToParent(src); BRect dest = parentsrc.OffsetByCopy(aDx, aDy); mView->Parent()->ConstrainClippingRegion(0); if(parentsrc.IsValid() && dest.IsValid()) mView->Parent()->CopyBits(src, dest); invalid.Exclude(mView->ConvertToParent(child->Frame())); invalid.Include(dest); int32 rects = invalid.CountRects(); for(int32 i = 0; i < rects; i++) { BRect curr = invalid.RectAt(i); nsRect r; r.x = (nscoord)curr.left; r.y = (nscoord)curr.top; r.width = (nscoord)curr.Width() + 1; r.height = (nscoord)curr.Height() + 1; OnPaint(r); } mView->UnlockLooper(); } Main idea here is performing copybits operation for Parent() if scrolling-view instead on scrolling-view itself. Those iFrames children appear on corect place and work as links in correct place too. Hope you have time to test it, i applied it together with my last patch for "disappearing images" bug.
After thousands of experiments i found only way to reduce i-frame lagging. It is: override BView::DrawAfterChildren; in ::Scroll, while moving children, apply B_DRAW_ON_CHILDREN flag. reset that flag after children were moved. Unfortunately those children, while don't lagging anymore, blink a bit. Especially when child's frame reaches parent frame border.
*** Bug 275380 has been marked as a duplicate of this bug. ***
(In reply to comment #11) > After thousands of experiments i found only way to reduce i-frame lagging. > It is: > override BView::DrawAfterChildren; > in ::Scroll, while moving children, apply B_DRAW_ON_CHILDREN flag. > reset that flag after children were moved. > > Unfortunately those children, while don't lagging anymore, blink a bit. > Especially when child's frame reaches parent frame border. Sergei, seems like this may have been taken care of as part of another bug/patch. Is this still a valid bug to have open?
The blinking is still there.
(In reply to comment #14) > The blinking is still there. True, it seems to be. But doesn't that mean the code discussed here is already implemented in some other patch? Otherwise, we'd still have the streaks of garbage pixels discussed in the original report. So the original problem seems fixed and not because of a patch for this bug but for some other.
This bug is still valid. Both description and origin of problem. All measures we took were palliatives, so atm, we still have to choose between two evils - ugly lagging and irritating blinking.
Created attachment 197343 [details] [diff] [review] Scroll and Paint rewrite Patch. Scrolling changes: Hiding children widgets while scrolling, unhiding at ::Update(). Calculating in-screen rect. Painting dirty area at once, instead traversing rects - will be rewritten when NS_PAINT really process regions, instead taking frame of those. Painting changes: 1)nsViewBeOS::GetPaintingRegion() don't empty now whole pending drawing region, there is new method for that nsViewBeOS::Validate(BRect), we are calling it in ::OnPaint(). So this substraction affect both synchronous and asynchronous drawing. 2)::Ivalidate() group of methods now calls again BView::Draw() instead Invalidate() in case of asynchronous painting. As we calculate update rects correctly now in ::Scroll() - no more app_server involvement required. 3)Only eWindowType_child and _popup are drawing now. Includes changes in ::OnPaint() method, in ::StandardWindowCreate() and in ::CreateBeOSView() I wish someone test this on plain cvs build, as we (me and tqh) are tested that with very non-standard sources only.
I think we should reduce code in OnPaint by removing static NS_DEFINE_IID(kRenderingContextCID, NS_RENDERING_CONTEXT_CID); static NS_DEFINE_IID(kRenderingContextIID, NS_IRENDERING_CONTEXT_IID); if (NS_SUCCEEDED(CallCreateInstance(kRenderingContextCID, &event.renderingContext))) { event.renderingContext->Init(mContext, this); result = DispatchWindowEvent(&event); NS_RELEASE(event.renderingContext); result = PR_TRUE; } else result = PR_FALSE; and replacing it with event.renderingContext = GetRenderingContext(); if(event.renderingContext != nsnull) { result = DispatchWindowEvent(&event); NS_RELEASE(event.renderingContext); } else { result = PR_FALSE; }
tqh, you didn't answer to my notice before about event.renderingContext = GetRenderingContext(); maybe you missed it. Ias far as i investigated it, GetRenderingContext in our case did absolutely same as we do, aonly adding call overhead. Maybe it was changed, but at least you can point me at current GetRenderingContext implementation
patch applies cleanly to CVS but breaks build. Also verified middle-button scrolling icon leaves a trail.
Doug, what is version you used to patch? CVS may provide various versions. Post full info from About page
Comment on attachment 197343 [details] [diff] [review] Scroll and Paint rewrite found bug in Validate() declaration
Created attachment 197354 [details] [diff] [review] patch Same as previous but :Validate() implementation now follows declaration. Also unused variable in OnPaint removed. Waiting also for tqh's comment about GetRenderingContext() benefits: http://lxr.mozilla.org/seamonkey/source/widget/src/xpwidgets/nsBaseWidget.cpp#605 http://lxr.mozilla.org/seamonkey/source/gfx/src/nsDeviceContext.cpp#220 (we lack proprietary implemetation atm) do_CreateInstance(kRenderingContextCID, &rv); - couldn't find code for that, only http://lxr.mozilla.org/seamonkey/source/xpcom/glue/nsGenericFactory.cpp#75
second patch version applies cleanly to CVS Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.9a1) Gecko/20050925 Firefox/1.6a1 fyysik, I hope this answers question about the version.
second patch version scrolls without flicker on pages with iFrames. Center-button scroll mouse trails are a separate problem - happen with CVS version and also after patching.
Fyysik, the only reason for getRenderingContext is so that we don't increase code-size with duplicate functionality and if something is changed in GetRenderingContext we don't have to do the same changes. Btw, the overhead is unnoticable for me.
Created attachment 197417 [details] [diff] [review] patch Same as above but: 1)sibling clipping check in ::Scroll() added 2)nsViewBeOS::DrawAferChildren() removed 3)Begin/EndResizingChildren emptied (to implement) 4)Added nsIRegion painting (mUpdateArea in *.h) to Scroll(), OnPaint(), InvalidateRegion(), CallMethod()ONPAINT: 5)GetRenderingContext() used in OnPaint() Hope tigerdog and tqh's will test it soon.
downloaded nsWindow from CVS, applied 3rd version of patch. anomalies with chatzilla and 3rd button mouse-scroll icon are gone. iFrame scrolling is much cleaner than CVS version. Scrolling is much improved over current CVS.
Comment on attachment 197417 [details] [diff] [review] patch r=thesuckiestemail@yahoo.se Tested it and looked at the code, much better than before.
It would be good if we could add this to the 1.8 branch as well. So that it is included in Firefox 1.5 release.
patch commited into trunk: nsWindow.cpp new revision: 1.95; previous revision: 1.94 nsWindow.h new revision: 1.36; previous revision: 1.35 Nielx, can you take care about branch?
Comment on attachment 197417 [details] [diff] [review] patch This patch touches BeOS-only files and therefore shouldn't be influencing the quality of any other platform. Patch revieuwed, working verified and approval will be highly appreciated.
Adding an i-framed forum that is really good for testing this, even though it might be the competitor :)
Comment on attachment 197417 [details] [diff] [review] patch Approved per 9/26 bug triage meeting.
or
