Closed Bug 157303 Opened 22 years ago Closed 21 years ago

Mozilla for BeOS doesn't display images on many sites.

Categories

(Core Graveyard :: GFX: BeOS, defect)

x86
BeOS
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Zaranthos_99, Assigned: beos)

References

Details

(Keywords: qawanted)

Attachments

(2 files, 5 obsolete files)

Mozilla for BeOS doesn't display images on many web sites. I tested several builds in BeOS and they all exibit the same problem. Mozilla 1.0 in Linux doesn't have this problem. I'll list a few sites that have this problem. http://rus.delfi.ee https://www.halifax-online.co.uk/_mem_bin/UMC001.asp Zaranthos: ah, fill in the fields, and when you tab to the button it vanishes http://www.pcworld.co.uk http://www.nvidia.com http://www.asus.com
Priority: -- → P5
Target Milestone: --- → mozilla1.1beta
tried to clean the cache or so? Reproducing a BeOS Bug is a bit complicate...
Tabbing to a button may make the image appear/disappear. Scrolling the screen up/down make make an image appear/disappear. Moving the mouse over an image make make it appear/disappear as will reloading a page sometimes. Clearing the cache doesn't help. The problem can be reproduced very easily on many sites and happens the first time or everytime you visit the problem sites.
please don't set the target milestone or the priority unless you are the assigned devloper - thanks CC arougthopher@lizardland.net for BEOS
Keywords: qawanted
Priority: P5 → --
Target Milestone: mozilla1.1beta → ---
Over to ImageLib.
Assignee: Matti → pavlov
Component: Browser-General → ImageLib
QA Contact: asa → tpreston
problem is not that it don't show them at all, rather than those images disappears. Or appears on some user actions, and then disappears again.
Installed BeOS here and I can reproduce the problem with a recent Mozilla (2002071310).
Status: UNCONFIRMED → NEW
Ever confirmed: true
hm.. I have an "image ghost" here.. some images are displayed as being missing, but loading them manually let them show up. The you go back to the page and they are there. Shitf+reload -> image gone again... same bug?
Yes, Kai, that is this bug
It seems there are different reasons for those sites. With last BeOS patches and maybe changes in above mozilla code, site *.delfi.ee now WORKSFORME. Other mentioned here still have problems
I have a feeling this is a network issues. This is why Sergei does not see the problem, since he is running BONE, which has a better network configuration. BeOS net_server is crappy, to say the least. My hunch is that this is within NSPR as well (maybe a blocking issue), but have not been able to verify this. There is currently another bug, which I think has a patch that I have not had a chance to test, for getting PRLog to a file working under BeOS again. Once I have done that, I need to add some logging to the BeOS networking code, and try to figure out what is going on in there with these images. I can almost always see this on http://www.osnews.com, in the icons for the articles. It seems as if mozilla gives up trying to load the image too quickly.
I see all those issues under BONE too. Just one site from examples works bit better for me now and i really doubt that it is (only) network-related. I think that this problem needs also ivestigation of code for those pages.
http://www.beosjournal.org is the latest victim of the image not displaying bug.
Reminder - should study code from http://www.3rd-evolution.de/BeOS/MDI/
Bug #1: code for ContainsRect in nsRegionBeOS is wrong. Intersects != Contains !!!!
from the nsIRegion.h file:/** * does the region intersect the rectangle? * * @param rect to check for containment * @return true if the region intersects the rect * **/ virtual PRBool ContainsRect(PRInt32 aX, PRInt32 aY, PRInt32 aWidth, PRInt32 aHeight) = 0;So, do we believe the comment or the method name? If the comment, then our implementationis correct. If we take the method literally, then it must be fully contained within the rect.
Nice find Paul - I was just about to post the same thing.For reference:- qt calls mRegion.contains(rect);- windows calls RectInRegion(mRegion, &trect)- mac calls RectInRgn(&macRect, mRegion)
So to summarize where we're at, the problem can temporarily be fixed by removing the call to ConstrainClippingRegion in nsRenderingContextBeOS::UpdateView(). In this case, no drawing disappears, but looking at drop down boxes, list views, etc. there are visible problems where the drawing hasn't been clipped. So for some reason the clipping region is occasionally wrong. One thing I tried was calling ConstrainClippingRegion(NULL) in UpdateView() before applying the new region, but this didn't work. Technically though, it looks like it would be necessary - otherwise you would be further reducing the current region with the new one (basically intersecting them). Could someone please check whether the clipping is taking place in the correct units (i.e. pixels)?
I was dumping the BRegion last night, to see if anything was a little "odd". I did not notice anything, but, the values did seem to be in pixels.
I will try to implement missing IsEqual, and ContainsRect as "total inclusive" check to see what happens. Though, Be API for regions is quite poor. So it may need some work. Btw, previous playing with ContainsRect solved here problems with: http://rus.delfi.ee and https://www.halifax-online.co.uk/_mem_bin/UMC001.asp (missing "statical" buttons, so defects appearing on redraw only (like scrolling) but not for renmaining sites which seems using (JS) layers or some other more sofisticated technique
When I was dumping the information about each region. NONE of the regions were complex, i.e. only single rectangles.
OK, my fellows! It seems that bug is found! Problem is with "nullregion". It is valid nsRegion with aX==aY==aWidth==aHeight==0. But in BRegion it results in region with single BRect with Frame().Width()==Frame().Height() = -1; Thats not so big problem, but problem (and there is remedy) is nsRegionBeOS::SetRegionType() implementation - it autoassigns mRegion with RectsCount == 1 to type eRegionComplexity_rect which is definitely wrong. Setting it for this case to eRegionComplexity_empty fixes our problem. So i'm going to check if there any side-effects and submitting patch after that (there are lot of other changes in my code, but only that one was definitive - so trying with untouched nsRegionBeOS.cpp).
Sorry, don't care about previous post. Sending it to /dev/null. My mistake - in reality i did same thing as Daniel - disable SetClippingRegion at all. But i have question - is possible that Mozilla adds rectangle to region originating from another view? P.S. Maybe we need better place to communicate:(
Addition to comment http://bugzilla.mozilla.org/show_bug.cgi?id=157303#c17 Replacing mView->ConstrainClippingRegion(region); with mView->ConstrainClippingRegion(0); fixes both problems - disappearing images, and artifacts in menus and drop-downs. (Also seems cured issue with stalling scrolling down in big bookmarks menu) (As far as i remember we are using ConstrainClippingRegion() in 3 more places - CopyBits in nsRenderingContextBeOS.cpp and in OnPaint() + Scroll() in nsWindowBeOS.cpp maybe some of them is overhead?
Paul, Daniel, can you test? - solution from previous comment (http://bugzilla.mozilla.org/show_bug.cgi?id=157303#c23) fixed also famous long-time bug about buggy scrollbars here (http://bugzilla.mozilla.org/show_bug.cgi?id=95348 ) If it is case for you, we should set dependency or duplication.
Sergei, this does the same thing as my change (commenting out the line completely). Either way, the drawing problems disappear, BUT new problems come up. For example, go to the Apple online store and try to configure a PowerBook, for example. You'll notice that if you play with the drop down boxes there to choose different options, sometimes the string will draw outside of its bounds. I've seen similar problems with text boxes on other sites too. In other words, turning off clipping completely is not a good solution either. We need to fix the clipping region instead.
Daniel, it may be not just the same, because in my case it cleans ClippingRegion, which maybe was set by 3 other instances of ConstrainClipping Region mentioned above. So i don't see yet any such hard issues. But maybe we can do something alike: in UpdateView: if(!mClipRegion)
Daniel, it may be not just the same, because in my case it cleans ClippingRegion, which maybe was set by 3 other instances of ConstrainClipping Region mentioned above. So i don't see yet any such hard issues. But maybe we can do something alike: in UpdateView: if(!mClipRegion) mView->ConstrainClippingRegion(0); instead existing code, and set clipping in nsRenderingContextBeOS::SetClipRegion in Windows port manner - something alike: NS_IMETHODIMP nsRenderingContextBeOS :: SetClipRegion(const nsIRegion& aRegion, nsClipCombine aCombine, PRBool &aClipEmpty) { BRegion *rgn, *currgn; rgn = currgn = nsnull; int cmode, cliptype; aRegion.GetNativeRegion((void *&)rgn); if (rgn && currgn && mView) { mView->GetClippingRegion(currgn); switch (aCombine) { case nsClipCombine_kIntersect: currgn->IntersectWith(rgn); break; case nsClipCombine_kUnion: currgn->Include(rgn); break; case nsClipCombine_kSubtract: currgn->Exclude(rgn); break; default: case nsClipCombine_kReplace: currgn = rgn; break; } PushState(); mView->ConstrainClippingRegion(currgn); } else return PR_FALSE; if (currgn->Frame().IsValid()) aClipEmpty = PR_TRUE; else aClipEmpty = PR_FALSE; return NS_OK; } instead existing implementation. P.S. Which browser you use? Your posts are non-wrapped, like ones posted from NetPositive.
Daniel, little notice and question. With code as is (ConstrainClippingRegion(region), obscured images appears when you MOVE some other window over this disappeared image. But don't if yoy hide Mozilla under some other window, and then activate Mozilla again. As i understand, Clipping region is thing for AppServer to deal with. But in one case it affects redrawing, in other don't. So maybe nsRegion code is ok, but we just don't call UpdateView when it supposed to be called, so it don't update ClippingRegion via UpdateView when it supposed, or just ignore some events?
addition to app_server notice (my previous comments): This code in UpdateView - if (mClipRegion ) { mClipRegion->GetNativeRegion((void *&)region); if(mView->Flags()&B_WILL_DRAW) mView->ConstrainClippingRegion(region); else mView->ConstrainClippingRegion(0); } does job very nice for me - no obscured images, bo artifacts, nice listboxes in forms
I have suspects that reported in some other bugs disappearing caret in text fiels (not the bug about too lefty 0-position) may be also related to this problem.
Attached patch Patch (obsolete) (deleted) — Splinter Review
This is a patch based on Sergei's comments. It really does seem to work. I have not seen any problems, and, works with the test Daniel had sent me. And yes, it does seem to fix the missing caret problem. (I've tried my damndest to make it diappear, and it hasn't yet :-) )
Paul, a few problems here. First, there's no reason to get the native region if you're going to be setting it to NULL. Second, there's a typo in ConstrainClippingRegion(0) in your patch. But the big problem is that some drawing is no longer clipped. Try choosing an optional display from the Apple Store when configuring a PowerBook for example.
Attached image Screen shot (deleted) —
This shows that text was not clipped properly in both the drop down box for choosing a monitor, as well as the top URL bar.
I also noticed problems with text in URL and some other places, Daniel, but that is more weird problem than you imagine. Text now don't go to wrap in some situation where it supposed to do so. It looks like that some boundaries were reported according to native clip region set (in some other place in Mozilla "higher-level" code), and maybe this is the reason for missing wrapping.
Little addition about this "non-hidden" text. It behaves now JUST in oppoite way to disappeared images in comment http://bugzilla.mozilla.org/show_bug.cgi?id=157303#c28 - if you move SLOWLY some window over this "extra" text - you can erase it, so all looks OK. But you cannot erase it by obscuring under another window - and erased text goes visible again when you bring Mozilla window to front. And also - you cannot erasi it totally, if you move "eraser" window too fast.
I wish Daniel provide me with other than URL-bar and AppleStore examples for exceeding text arising in case of patch. Both are same case - very annoying for BeOS hybrid object - drop-down box. Problem is - those objects consist of BView belonging to top-level window and separate window (created as eWindowType_popup) window. (So if you look at BWindows list in AplleStore case, as for any window with drop-down boxes, e,g, on BeZilla page, you can see bunch of w-> threads) I have feeling that here appears again famous contradiction between BeOS and remaining world - windows are in separate thread (and so at different loopers), and popup window don't belong to window where "first item of list" is placed. And i'm afraid that somewhere it wasn't took in account, so some Mozilla's paint/update/drawing-setting message pass to such object only once, for object as whole. Which is insufficient in BeOS case.
Found "ideal" solution for nsRenderingContextBeOS. Just like each UpdateView() is balanced with UnlockLooper(), same way (almost) each UpdateView should be also balanced with ConstrainClippingRegion(0) in drawing primitives (befor UnlockLooper). No disappeared images, no strings with broken Z-order. Patch http://bugzilla.mozilla.org/attachment.cgi?id=109359&action=view should be removed/obsoleted
Comment on attachment 109359 [details] [diff] [review] Patch obsoleting
Attachment #109359 - Attachment is obsolete: true
Attached patch patch for testing (obsolete) (deleted) — Splinter Review
Creating pair for UpdateView() - UnlockViewLooper() and replacing mView->UnlockLooper() with it. Though, i dislike current form and name of UpdateView - i prefer something like bool LockAndSetView(BView *) { bool retvalue; **** if((retvalue =(mView && mView->LockLooper))) { **** } **** return retvalue; } and same for this new function.
New inestigation (as comment for previous patch) - it seems that ContstrainClippingRegion(0) before UnlockLooper is needed only in NS_IMETHODIMP nsRenderingContextBeOS::DrawString(const char *aString, PRUint32 aLength, nscoord aX, nscoord aY, const nscoord *aSpacing). It seems related to double draw mode change in this function.
Ohh, it appears that since spring of 2001 DrawImage and DrawTile is outside nsRenderingContext in reality. in ns*Image.cpp. Which does sets clipping and other things independently. And hasn't UpdateView analog. I suspect some collisions there.
Attached patch Patch for DrawString (obsolete) (deleted) — Splinter Review
Anyway, submitting patch which fixes mentioned problems for me. Inspite it seems more like hack.
Attachment #109825 - Attachment is obsolete: true
it seems there really are another clipping mistakes, supposedly related to nsImage.cpp methods - for example on http://lingvo.yandex.ru background image for submit buttons (on right) spreads itself above this button even with applied patch.
Not just buttons either with not enough clipping. Like one of the bugs you've linked to, images in table cells sometimes spread outside of their bounding boxes (the fire animation in the tinderbox does this, as well as viewer demos 3 and 4). This is on a trunk nightly from 4 December, so no patches. Also, I don't like the ConstrainClippingRegion(0) thing - it feels like a hack to me. BeOS gfx is still slow, and there are now quite a few bugs (scrolling, disappearing images, also -moz-opacity seems to make things completely invisible). What we really need is a full rewrite, after deciding on a much more efficient looper-locking mechanism. We can implement it with just speed in mind (using aFontID and GetWidth and things to speed rendering, for example). Plus, the bugs with clipping would probably be fixed during this process. What does anyone think?
A rewrite would be nice, but the total lack of documentation makes that very difficult. It's the main reason I'm not contributing to the project anymore. There is no explanation of how the graphics subsystem works, who owns what, how the double buffering and scrolling operate, what the coordinate system assumptions are, etc. All you get are one or two sentences above methods in the interface headers, things like "nsImage::Optimize() optimizes the image". Having said all that, I think that the BeOS widget code is even more broken than the gfx code. Text boxes, scrollbars, and drop downs all still have problems. Plus there's no drag-and-drop, no printing, and other higher-level features that people miss. In my opinion, the disappearing images is a priority, but apart from that the gfx implementation is acceptable enough to leave alone for a while, and fix bugs elsewhere.
I know there isn't a lot of documentation, I've been trying to find it for the last couple of weeks. As you say the interface headers aren't particularly helpful (a bit like Microsoft's .net help: "IsNavigable sets whether or not the data grid is navigable"). However there are plenty of other platform gfx implementations to look at - most of the purpose of the functions can be guessed from this. Side note: is there a reference platform with everything implemented? GTK, Windows? I guess it depends on your system how important the gfx speed is. On my lowly K6/2 450 it is usable enough to be my fulltime browser, but a lot slower than moz on windows (or even IE). Slow enough that I have to quickly switch to a different workspace and fire up soundplay or something to remind me I'm running BeOS. Getting to the bottom of the images problem would be a good excuse to try a new, fast gfx implementation. There's plenty of other stuff that needs doing too, of course. But if we could get a 10x gfx speedup, I could live with scrollbars that sometimes appear when they shouldn't and not having any printing ;)
Anyway, i'm doing gfx revision now. Some things are obsolete, far behind time. E.g. in nsImageBeOS DrawToImage code was too old and caused big processor load on animated images. Also there were two version of DrawTile (background painting method) - and only one of them exists now in other ports and even in public definition. And more, seems here, in nsImageBeOS, is just reason for this bug: for quite long time Mozilla uses DrawImage and DrawTile from nsImage instead nsRenderingContext - and DrawImage in nsImageBeOS lacks ConstrainClippingRegion(rgn) (all drawing methods in nsRendering contain it via UpdateView). Once added, it fixed bug.
Does this fix all clipping problems - both disappearing images and images spreading outside their bounding box? What about text box carets? I guess this means ConstrainClippingRegion(0) is no longer needed. That would be great, I was never happy with that as a permenant fix.
1)I didn't see bugs with viewer demos 3 and 4 for long time 2) I don't use ConstrainClippingRegion(0) in UpdateView(). Only place where it was added in my code, as i noticed in one of comments - end of DrawString functions. But now i removed it even from there. 3) Patch for polygons fixed part of caret problem in single-live fields for me, but ther is another problem, related to focus/activation events, when caret sometimes don't appear on field on mouse click - it appears there after some keyboard input - and if you delete all input - cret is still visible in ultimate left position
"mass" re-assigning of bugs from pav to myself
Assignee: pavlov → jdunn
*** Bug 125477 has been marked as a duplicate of this bug. ***
Patch for http://bugzilla.mozilla.org/show_bug.cgi?id=201624 obsoleted this problem. Mozilla now uses XUL-scrollbars. But problem is about status - should it be FIXED ? WONTFIX?
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
oops, closed wrong bug, reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 218134
*** Bug 201536 has been marked as a duplicate of this bug. ***
Attached patch very important patch (obsolete) (deleted) — Splinter Review
Fixing this most long-standing, most awful and annoying BeOS GFX bug. I think there are lot of several dups of it on bugzilla. Problem was explained in http://bugzilla.mozilla.org/show_bug.cgi?id=157303#c41 To solve problem in unified with nsRenderingContextBeOS way, i made former UpdateView() public (like UpdateGC in gtk port), changed it to bool (in order to return test results for mView existance and its locked state) and changed this function name to LockAndUpdateView(). Then used this LockAndUpdateView() in nsImageBeOS Draw() and DrawTile() methods.
Attachment #110178 - Attachment is obsolete: true
Comment on attachment 138338 [details] [diff] [review] very important patch review request
Attachment #138338 - Flags: review?(timeless)
Patch seems to solve many graphics problems. Works much better when scrolling. No redraw-problems. Couldn't spot any rendering problems at all in fact.
Comment on attachment 138338 [details] [diff] [review] very important patch + //LockAndUpdate() sets proper clipping region here and elsewhere in nsImageBeOS. + if(((nsRenderingContextBeOS&)aContext).LockAndUpdateView()) { the comment should say LockAndUpdateView, not LockAndUpdate, apparently } beosdrawing->ReleaseView(); + } is this a -w patch? otherwise, the indentation here is wrong.
biesi, yeah, i did diff -uwp. Should i use different diff flags set?
Attachment #138338 - Attachment filename: nsImageBeOS.patch → nsImageBeOS.patch, diff -uwp
Assignee: jdunn → arougthopher
Status: REOPENED → NEW
Component: ImageLib → GFX: BeOS
QA Contact: tpreston → timeless
Attached patch Patch (diff -up) (obsolete) (deleted) — Splinter Review
Comment changed. type of LockAndUpdateView() changing to bool (to be compatible with BView::LockLooper in return value and to look C/C++ "native" when used in if()).
Attachment #138338 - Attachment is obsolete: true
Comment on attachment 138338 [details] [diff] [review] very important patch removing review request from obsoleted patch
Attachment #138338 - Flags: review?(timeless)
Comment on attachment 138385 [details] [diff] [review] Patch (diff -up) review request
Attachment #138385 - Flags: review?(timeless)
Attached patch patch -up with new comment (deleted) — Splinter Review
Added large comment (by biesi request) about LockAndUpdateView() method
Attachment #138385 - Attachment is obsolete: true
Comment on attachment 138389 [details] [diff] [review] patch -up with new comment marking r=timeless from irc
Attachment #138389 - Flags: review+
Status: NEW → RESOLVED
Closed: 22 years ago21 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

Creator:
Created:
Updated:
Size: