Bug 104550 - [Qt] MediaPlayerQTKit broken after switching QPainter to raster engine.
Summary: [Qt] MediaPlayerQTKit broken after switching QPainter to raster engine.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zeno Albisser
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-10 06:40 PST by Zeno Albisser
Modified: 2012-12-19 04:35 PST (History)
7 users (show)

See Also:


Attachments
Patch (4.56 KB, patch)
2012-12-10 06:45 PST, Zeno Albisser
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zeno Albisser 2012-12-10 06:40:11 PST
[Qt] MediaPlayerQTKit broken after switching QPainter to raster engine.
Comment 1 Zeno Albisser 2012-12-10 06:45:45 PST
Created attachment 178539 [details]
Patch
Comment 2 Simon Hausmann 2012-12-10 07:23:45 PST
Comment on attachment 178539 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=178539&action=review

r=me with comments/changes.

Looking forward to this code path disappearing altogether hopefully in the near future ;-)

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1374
> +    NSBitmapImageRep *imageRep = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes: NULL

I think you're going to need to call [imageRep release] later on after painting to release the memory again, no?
Comment 3 Zeno Albisser 2012-12-10 07:39:04 PST
(In reply to comment #2)
> Looking forward to this code path disappearing altogether hopefully in the near future ;-)
Me too.

> > Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1374
> > +    NSBitmapImageRep *imageRep = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes: NULL
> 
> I think you're going to need to call [imageRep release] later on after painting to release the memory again, no?
Yes.
Comment 4 Zeno Albisser 2012-12-10 08:12:11 PST
Committed r137159: <http://trac.webkit.org/changeset/137159>
Comment 5 Balazs Kelemen 2012-12-19 02:50:53 PST
Just a quick note on this patch: probably we want to introduce a global (WTF) helper for bgr <-> rgb conversion, since it is subject of SIMD and I believe Gabor Rapcsányi already implemented it in NEON at some other place :)
Comment 6 Simon Hausmann 2012-12-19 04:30:46 PST
(In reply to comment #5)
> Just a quick note on this patch: probably we want to introduce a global (WTF) helper for bgr <-> rgb conversion, since it is subject of SIMD and I believe Gabor Rapcsányi already implemented it in NEON at some other place :)

Maybe yes, but we're going to stop using the code here very soon, so that's going to be one user less.
Comment 7 Gabor Rapcsanyi 2012-12-19 04:35:28 PST
(In reply to comment #5)
> Just a quick note on this patch: probably we want to introduce a global (WTF) helper for bgr <-> rgb conversion, since it is subject of SIMD and I believe Gabor Rapcsányi already implemented it in NEON at some other place :)

I had a patch to speed up a little bit this conversion:
https://bugs.webkit.org/show_bug.cgi?id=103630

With NEON SIMD it could be twice as fast as now, if you are interested in I can make a bug for it.