Closed Bug 239813 Opened 21 years ago Closed 20 years ago

BeOS Transparency: nsDrawingSurface::Lock() needs implementing

Categories

(Core Graveyard :: GFX: BeOS, defect)

x86
BeOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: simontaylor2, Assigned: sergei_d)

References

Details

Attachments

(2 files, 2 obsolete files)

Bug 206561 also affects BeOS: http://bugzilla.mozilla.org/show_bug.cgi?id=206561 Worse, not freeing offscreen bitmaps soon brings down the whole system, as each offscreen bitmap that is drawable uses it's own thread in BeOS, and the app_server runs out of threads quite quickly. The fix is the same as in the GTK code. Also, BeOS has never had an implementation of nsDrawingSurface::Lock() and Unlock(), which means sites with transparency (and the new firefox download manager) don't render correctly as well as bringing the system down as described above. I have an implementation for this too. Patch to follow...
Attached patch patch (diff -up4) (obsolete) (deleted) — Splinter Review
Fixes leak of offscreen bitmaps (and the whole-system crash that caused), and adds an implementation for nsDrawingSurface::Lock() and Unlock() so sites that use -opacity and the Firefox download manager render properly.
Comment on attachment 145553 [details] [diff] [review] patch (diff -up4) Reveiw request - BeOS-specific code in BeOS-specific folder. Sergei can you review this?
Attachment #145553 - Flags: review?(sergei_d)
Seems good so far, though, before reviewing and checkin i wish to ask you opinion on problems i'm unsure myself in. 1)Shouldn't we in destructor in if(mBitmap) case also check for for mView!=0 and also remove it, not only detach from mBitmap? As in case of mBitmap-ped surface this is temporary view belonging to that BBitmap only - so cleanup for safety. 2)Looper and Bits locking. I'm unsure myself at moment about locking policy. Should we lock looper and bits at all? And also, if it is reuired, should we lock/Unlock those objetcs in both nsSurface::Lock and nsSurface::Unlock. Or, just lock looper in nsSurfaceLock and unlock in nsSurface::Unlock? And maybe bits unlocking should depend on flags NS_LOCK_SURFACE_READ_ONLY and NS_LOCK_SURFACE_WRITE_ONLY ? Honestly, i don't know:( 3)I noticed that gtk implementation has special care about deleting mLockBitmap in nsSurface::Init. What do you think about it?
Summary: Transparency issues: no nsDrawingSurface::Lock(), and memory leak → Transparency issues: no nsDrawingSurface::Lock(), and memory leak
patch is outdated anyway, as leak fix happened in other bug already
Comment on attachment 145553 [details] [diff] [review] patch (diff -up4) Obseleting patch
Attachment #145553 - Attachment is obsolete: true
Leak fixed in another bug, no longer critical
Severity: critical → normal
Blocks: 266252
Attached patch Patch for 1.0.0-d4 (tangobravo (obsolete) (deleted) — Splinter Review
This is the patch taht will be included in 1.0.0-d4
Changed summary to just be about transparency implementation
Summary: Transparency issues: no nsDrawingSurface::Lock(), and memory leak → BeOS Transparency: nsDrawingSurface::Lock() needs implementing
Comment on attachment 165203 [details] [diff] [review] Patch for 1.0.0-d4 (tangobravo obsoleting
Attachment #165203 - Attachment is obsolete: true
Assignee: beos → sergei_d
Attached patch patch (deleted) — Splinter Review
Slightly cleaned previous version. Time to checkin
Attachment #174858 - Flags: review?(thesuckiestemail)
Comment on attachment 174858 [details] [diff] [review] patch Some mix of tabs and spaces, but I can't see anything else wrong.
Attachment #174858 - Flags: review?(thesuckiestemail) → review+
checked in 2005-02-20 09:15 "Bug 239813 - implementing nsDrawingSurface::Lock()/Unlock() for BeOS." with little formatting cleanup Marking as fixed
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
It seems we don't need that mess with temporary bitmap, providing proper pointer and stride for existing backbuffer bitmap looks totally sufficient to perform required task.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch for simplified solution (deleted) — Splinter Review
It works here as good as previous version. Only issue is that we lack in BeOS any method to really "lock" certain part of BBitmap, so if something tries to write to "lock rect" at backbuffer, nothing prevents from that action. (With additional bitmap we preserved content of that rect). But actually backbuffer bitmap is always locked for use by certain thread only for lifetime, so i doubt that it may be issue
Attachment #175172 - Flags: review?(thesuckiestemail)
Comment on attachment 175172 [details] [diff] [review] patch for simplified solution r=thesuckiestemail@yahoo.se
Attachment #175172 - Flags: review?(thesuckiestemail) → review+
simpler version checked in. also removed unused lock-variables from nsDrawingSurfaceBeOS.h
Status: REOPENED → RESOLVED
Closed: 20 years ago20 years ago
Resolution: --- → FIXED
Another quick checkin! Hope it works properly, I'll be testing soon.
Attachment #145553 - Flags: review?(sergei_d)
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: