Closed
Bug 959154
Opened 10 years ago
Closed 10 years ago
Moz2Dify GLUploadHelpers
Categories
(Core :: Graphics, defect)
Core
Graphics
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: pehrsons, Assigned: pehrsons)
References
Details
Attachments
(8 files, 3 obsolete files)
1.39 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
4.38 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
8.43 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
10.00 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
10.41 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
7.68 KB,
patch
|
nical
:
review+
|
Details | Diff | Splinter Review |
26.55 KB,
patch
|
Details | Diff | Splinter Review | |
8.55 KB,
patch
|
Details | Diff | Splinter Review |
Used to be a part of GLContext, but bug 877115 is getting bloated by comments and patches so let's break distinguishable parts out of it.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee | ||
Comment 2•10 years ago
|
||
Assignee | ||
Comment 3•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8375569 -
Attachment is obsolete: true
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Assignee | ||
Comment 9•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8375568 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8375570 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8375572 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8375576 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8375578 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8375579 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8375581 [details] [diff] [review] Part 7. Moz2Dify GLTextureImage::BeginUpdate/EndUpdate. I am not sure of the implications to removing the SetDeviceOffset calls when moving from cairo surfaces to draw targets. Please enlighten me. It didn't seem to cause any new reftest failures on my local build so I went ahead. I'll launch a full test run on try soon as well.
Attachment #8375581 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8375582 [details] [diff] [review] Part 8. Moz2Dify GLUploadHelpers. Ah, joy!
Attachment #8375582 -
Flags: review?(nical.bugzilla)
Assignee | ||
Comment 12•10 years ago
|
||
try push: https://tbpl.mozilla.org/?tree=Try&rev=b840817a28c2
Updated•10 years ago
|
Attachment #8375568 -
Flags: review?(nical.bugzilla) → review+
Updated•10 years ago
|
Attachment #8375570 -
Flags: review?(nical.bugzilla) → review+
Updated•10 years ago
|
Attachment #8375572 -
Flags: review?(nical.bugzilla) → review+
Updated•10 years ago
|
Attachment #8375576 -
Flags: review?(nical.bugzilla) → review+
Updated•10 years ago
|
Attachment #8375578 -
Flags: review?(nical.bugzilla) → review+
Updated•10 years ago
|
Attachment #8375579 -
Flags: review?(nical.bugzilla) → review+
Comment 13•10 years ago
|
||
Comment on attachment 8375581 [details] [diff] [review] Part 7. Moz2Dify GLTextureImage::BeginUpdate/EndUpdate. Review of attachment 8375581 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLTextureImage.cpp @@ +147,3 @@ > > + if (!mUpdateDrawTarget) { > + mUpdateDrawTarget = nullptr; nit: if mUpdateDrawTarget is null you don't really need to nullify it even more :)
Attachment #8375581 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 14•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8375581 -
Attachment is obsolete: true
Assignee | ||
Comment 15•10 years ago
|
||
Comment on attachment 8375646 [details] [diff] [review] Part 7. Moz2Dify GLTextureImage::BeginUpdate/EndUpdate. (carries r=nical) Hmm, I agree ;)
Attachment #8375646 -
Attachment description: Part 7. Moz2Dify GLTextureImage::BeginUpdate/EndUpdate. → Part 7. Moz2Dify GLTextureImage::BeginUpdate/EndUpdate. (carries r=nical)
Updated•10 years ago
|
Attachment #8375582 -
Flags: review?(nical.bugzilla) → review+
Assignee | ||
Comment 16•10 years ago
|
||
Some reftest failures indeed. I seem to have mixed up blue with red somehow in part 1. I can reproduce the failures locally, although mach claims all these tests were good I can see them being wrong as they flicker by. Weird, but good enough for me.
Assignee | ||
Comment 17•10 years ago
|
||
Comment on attachment 8375568 [details] [diff] [review] Part 1. Moz2Dify gl::UploadImageData. Review of attachment 8375568 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/GLUploadHelpers.cpp @@ +422,3 @@ > if (gl->GetPreferredARGB32Format() == LOCAL_GL_BGRA) { > format = LOCAL_GL_BGRA; > + surfaceFormat = SurfaceFormat::B8G8R8A8; to this. Well. Looks suspicious :) @@ -421,3 @@ > if (gl->GetPreferredARGB32Format() == LOCAL_GL_BGRA) { > format = LOCAL_GL_BGRA; > - surfaceFormat = gfx::SurfaceFormat::R8G8B8A8; From this,
Assignee | ||
Comment 18•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8375568 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8376185 -
Attachment description: Part 1. Moz2Dify gl::UploadImageData. → Part 1. Moz2Dify gl::UploadImageData. (carries r=nical)
Assignee | ||
Comment 19•10 years ago
|
||
try push: https://tbpl.mozilla.org/?tree=Try&rev=bcf526e2565b
Assignee | ||
Comment 20•10 years ago
|
||
Looks good now. I'll wait for 10.8 to finish the tests before checking in.
Assignee | ||
Comment 21•10 years ago
|
||
The 10.8 reftest run doesn't even seem to start. Since they were exactly the same as the 10.6 failures before, I'll go ahead with checkin.
Keywords: checkin-needed
Comment 22•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fffb200a94bc https://hg.mozilla.org/integration/mozilla-inbound/rev/b2535f63d78e https://hg.mozilla.org/integration/mozilla-inbound/rev/88a6337fc8fb https://hg.mozilla.org/integration/mozilla-inbound/rev/53da6aef5629 https://hg.mozilla.org/integration/mozilla-inbound/rev/c6ec9df2d729 https://hg.mozilla.org/integration/mozilla-inbound/rev/6ca9ba706c94 https://hg.mozilla.org/integration/mozilla-inbound/rev/0f9be8d2caaf https://hg.mozilla.org/integration/mozilla-inbound/rev/00e465be0552
Assignee: nobody → pehrsons
Keywords: checkin-needed
Comment 23•10 years ago
|
||
Backed out for various test failures: https://hg.mozilla.org/integration/mozilla-inbound/rev/5749f2fc4f47 https://tbpl.mozilla.org/php/getParsedLog.php?id=34693418&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=34693537&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=34693300&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=34694043&tree=Mozilla-Inbound
Assignee | ||
Comment 24•10 years ago
|
||
I believe I am innocent this time :) https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=314f24c02ea5 includes my patches and those oranges does not exist. They seem to be from the run before the backout (also caused by it) in that revision. Ryan (or other sheriff), I'm setting checkin-needed again and you can double check.
Keywords: checkin-needed
Comment 25•10 years ago
|
||
Correct, I was just coming here to say the same. https://hg.mozilla.org/integration/mozilla-inbound/rev/5dacf72427de https://hg.mozilla.org/integration/mozilla-inbound/rev/a48e28808790 https://hg.mozilla.org/integration/mozilla-inbound/rev/28ba7f53aa95 https://hg.mozilla.org/integration/mozilla-inbound/rev/2b36d265d7a0 https://hg.mozilla.org/integration/mozilla-inbound/rev/6ebe38da7602 https://hg.mozilla.org/integration/mozilla-inbound/rev/cac044190278 https://hg.mozilla.org/integration/mozilla-inbound/rev/4083df96bd0c https://hg.mozilla.org/integration/mozilla-inbound/rev/58afc23fabe8
Keywords: checkin-needed
Comment 26•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5dacf72427de https://hg.mozilla.org/mozilla-central/rev/a48e28808790 https://hg.mozilla.org/mozilla-central/rev/28ba7f53aa95 https://hg.mozilla.org/mozilla-central/rev/2b36d265d7a0 https://hg.mozilla.org/mozilla-central/rev/6ebe38da7602 https://hg.mozilla.org/mozilla-central/rev/cac044190278 https://hg.mozilla.org/mozilla-central/rev/4083df96bd0c https://hg.mozilla.org/mozilla-central/rev/58afc23fabe8
Status: UNCONFIRMED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•