Upgrading Mesa to 9.0.3

Upgrading Mesa to 9.0.3

BUG=238755
R=apatrick@chromium.org, nduca@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209579

[Ken Russell (switch to Gerrit), 2013-06-18 22:29:41]

One issue to be resolved.

https://chromiumcodereview.appspot.com/17005007/diff/6001/ui/gl/gl.gyp
File ui/gl/gl.gyp (right):

https://chromiumcodereview.appspot.com/17005007/diff/6001/ui/gl/gl.gyp#newcode20
ui/gl/gl.gyp:20: '<(DEPTH)/third_party/khronos/khronos.gyp:khronos_headers',
Please undo the changes to this file. They shouldn't be necessary any more, and
if they are, then we need to find a different solution.

[Ken Russell (switch to Gerrit), 2013-06-18 22:35:57]

Sorry, one other issue. I didn't realize that the try jobs would be rejected if
the patch contained binary files. Could you please upload a version of the patch
without the PNGs so we can send it to the try servers? We'll expect to see those
tests fail in the try runs, which is OK. At this point we're trying to tease out
build failures and crashes on platforms we didn't test, like Android and Chrome
OS.

[Dirk Pranke, 2013-06-18 22:51:56]

On 2013/06/18 22:35:57, kbr wrote:
> Sorry, one other issue. I didn't realize that the try jobs would be rejected
if
> the patch contained binary files. Could you please upload a version of the
patch
> without the PNGs so we can send it to the try servers? We'll expect to see
those
> tests fail in the try runs, which is OK. At this point we're trying to tease
out
> build failures and crashes on platforms we didn't test, like Android and
Chrome
> OS.

Myles uploaded a binary-less patch in https://codereview.chromium.org/17422003/
and those try jobs are running now.

[Myles C. Maxfield (Amazon.com), 2013-06-18 22:52:01]

Way ahead of you. I've been talking to Dirk Pranke about this on IRC.
https://codereview.chromium.org/17422003/

https://chromiumcodereview.appspot.com/17005007/diff/6001/ui/gl/gl.gyp
File ui/gl/gl.gyp (right):

https://chromiumcodereview.appspot.com/17005007/diff/6001/ui/gl/gl.gyp#newcode20
ui/gl/gl.gyp:20: '<(DEPTH)/third_party/khronos/khronos.gyp:khronos_headers',
They are necessary. I'll revert these and show you the relevant error, then we
can come up with a solution together.

On 2013/06/18 22:29:41, kbr wrote:
> Please undo the changes to this file. They shouldn't be necessary any more,
and
> if they are, then we need to find a different solution.

[Ken Russell (switch to Gerrit), 2013-06-18 23:06:45]

Great. Thanks Dirk and Myles.

[Myles C. Maxfield (Amazon.com), 2013-06-19 00:13:10]

On 2013/06/18 23:06:45, kbr wrote:
> Great. Thanks Dirk and Myles.

Alright, the error is below. It's because Chromium has modified
third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__) stanza,
which defines EGLNative*Type. I could make similar modifications to
third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding khronos
to the search path before mesa is a better solution.

-c ../../ui/gl/gl_context_osmesa.cc -o obj/ui/gl/gl.gl_context_osmesa.o
In file included from ../../ui/gl/gl_context_osmesa.cc:11:
In file included from ../../ui/gl/gl_bindings.h:15:
In file included from ../../third_party/mesa/src/include/EGL/egl.h:36:
../../third_party/mesa/src/include/EGL/eglplatform.h:128:2: error: "Platform not
recognized"
#error "Platform not recognized"
 ^
../../third_party/mesa/src/include/EGL/eglplatform.h:132:9: error: unknown type
name 'EGLNativeDisplayType'
typedef EGLNativeDisplayType NativeDisplayType;
        ^
../../third_party/mesa/src/include/EGL/eglplatform.h:133:9: error: unknown type
name 'EGLNativePixmapType'
typedef EGLNativePixmapType  NativePixmapType;
        ^
../../third_party/mesa/src/include/EGL/eglplatform.h:134:9: error: unknown type
name 'EGLNativeWindowType'
typedef EGLNativeWindowType  NativeWindowType;
        ^
In file included from ../../ui/gl/gl_context_osmesa.cc:11:
In file included from ../../ui/gl/gl_bindings.h:15:
../../third_party/mesa/src/include/EGL/egl.h:251:45: error: unknown type name
'EGLNativeDisplayType'
EGLAPI EGLDisplay EGLAPIENTRY eglGetDisplay(EGLNativeDisplayType display_id);
                                            ^
../../third_party/mesa/src/include/EGL/egl.h:266:7: error: unknown type name
'EGLNativeWindowType'
                                  EGLNativeWindowType win,
                                  ^
../../third_party/mesa/src/include/EGL/egl.h:271:7: error: unknown type name
'EGLNativePixmapType'
                                  EGLNativePixmapType pixmap,
                                  ^
../../third_party/mesa/src/include/EGL/egl.h:314:6: error: unknown type name
'EGLNativePixmapType'
                          EGLNativePixmapType target);
                          ^
8 errors generated.





Not only that, but there's another problem: the 'gl' target in ui/gl/gl.gyp
specifies 'third_party/mesa/src/include' in its 'include_dirs' stanza, but
doesn't actually depend on mesa_headers. This means that the
direct_dependent_settings stanza defined in the 'mesa_headers' target doesn't
actually get applied, which results in the "unknown pragma" error that we've
seen before. If you're interested, I can reproduce this as well.

--------------------

Separate issue: as you can see, almost all of the trybots trying out
https://codereview.chromium.org/17422003/ are red. I started specifically
looking at the mac_gpu_retina bots. In particular, Dirk ran these bots twice,
and the same set of 6 tests failed both times. However, on my own retina mac, 5
of the 6 tests pass, and the last one fails on ToT as well. In addition, I
believe that the gpu tests should be orthogonal to mesa, since osmesa is, by
definition, software-based, and doesn't touch the GPU. I believe the GPU tests
are testing the system OpenGL implementation, and therefore they shouldn't be
changed at all. I was hoping for some guidance about what you think about this,
Ken.

Thanks,
Myles

[Ken Russell (switch to Gerrit), 2013-06-19 00:41:31]

On 2013/06/19 00:13:10, Myles C. Maxfield (Amazon.com) wrote:
> 
> Alright, the error is below. It's because Chromium has modified
> third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__) stanza,
> which defines EGLNative*Type. I could make similar modifications to
> third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding khronos
> to the search path before mesa is a better solution.

Gregg Tavares pointed out to me offline that the third_party/khronos headers
have been modified to work with Chrome's command buffer implementation, and
aren't suitable for use when implementing the actual GL bindings. We have to
apply this patch to Mesa's eglplatform.h. Could you please submit a CL doing
this, and then resubmit the DEPS roll to the new revision?

> Not only that, but there's another problem: the 'gl' target in ui/gl/gl.gyp
> specifies 'third_party/mesa/src/include' in its 'include_dirs' stanza, but
> doesn't actually depend on mesa_headers. This means that the
> direct_dependent_settings stanza defined in the 'mesa_headers' target doesn't
> actually get applied, which results in the "unknown pragma" error that we've
> seen before. If you're interested, I can reproduce this as well.

I don't follow this --
http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates that the
top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
understand it your new suppressions in mesa.gyp:mesa_headers for unknown pragmas
should be taking effect.

> Separate issue: as you can see, almost all of the trybots trying out
> https://codereview.chromium.org/17422003/ are red. I started specifically
> looking at the mac_gpu_retina bots. In particular, Dirk ran these bots twice,
> and the same set of 6 tests failed both times. However, on my own retina mac,
5
> of the 6 tests pass, and the last one fails on ToT as well. In addition, I
> believe that the gpu tests should be orthogonal to mesa, since osmesa is, by
> definition, software-based, and doesn't touch the GPU. I believe the GPU tests
> are testing the system OpenGL implementation, and therefore they shouldn't be
> changed at all. I was hoping for some guidance about what you think about
this,
> Ken.

It's definitely possible that the _gpu bot try failures are unrelated to your
patch. zmo knows the state of the gpu try servers better than I do. Right now
we're just trying to get basic coverage of your patch on the majority of our
configurations.

Note that there are failures on linux_layout_rel and mac_layout_rel that
indicate more suppressions are needed in skia_test_expectations.txt; please look
through the try job results and apply any changes you think are needed. Feel
free to ping us about sending off try jobs against newer patches.

[Stephen White, 2013-06-19 01:40:43]

On 2013/06/19 00:41:31, kbr wrote:
> On 2013/06/19 00:13:10, Myles C. Maxfield (http://Amazon.com) wrote:
> > 
> > Alright, the error is below. It's because Chromium has modified
> > third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__)
stanza,
> > which defines EGLNative*Type. I could make similar modifications to
> > third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding
khronos
> > to the search path before mesa is a better solution.
> 
> Gregg Tavares pointed out to me offline that the third_party/khronos headers
> have been modified to work with Chrome's command buffer implementation, and
> aren't suitable for use when implementing the actual GL bindings. We have to
> apply this patch to Mesa's eglplatform.h. Could you please submit a CL doing
> this, and then resubmit the DEPS roll to the new revision?
> 
> > Not only that, but there's another problem: the 'gl' target in ui/gl/gl.gyp
> > specifies 'third_party/mesa/src/include' in its 'include_dirs' stanza, but
> > doesn't actually depend on mesa_headers. This means that the
> > direct_dependent_settings stanza defined in the 'mesa_headers' target
doesn't
> > actually get applied, which results in the "unknown pragma" error that we've
> > seen before. If you're interested, I can reproduce this as well.
> 
> I don't follow this --
> http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates that
the
> top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
> understand it your new suppressions in mesa.gyp:mesa_headers for unknown
pragmas
> should be taking effect.
> 
> > Separate issue: as you can see, almost all of the trybots trying out
> > https://codereview.chromium.org/17422003/ are red. I started specifically
> > looking at the mac_gpu_retina bots. In particular, Dirk ran these bots
twice,
> > and the same set of 6 tests failed both times. However, on my own retina
mac,
> 5
> > of the 6 tests pass, and the last one fails on ToT as well. In addition, I
> > believe that the gpu tests should be orthogonal to mesa, since osmesa is, by
> > definition, software-based, and doesn't touch the GPU. I believe the GPU
tests
> > are testing the system OpenGL implementation, and therefore they shouldn't
be
> > changed at all. I was hoping for some guidance about what you think about
> this,
> > Ken.
> 
> It's definitely possible that the _gpu bot try failures are unrelated to your
> patch. zmo knows the state of the gpu try servers better than I do. Right now
> we're just trying to get basic coverage of your patch on the majority of our
> configurations.
> 
> Note that there are failures on linux_layout_rel and mac_layout_rel that
> indicate more suppressions are needed in skia_test_expectations.txt; please
look
> through the try job results and apply any changes you think are needed. Feel
> free to ping us about sending off try jobs against newer patches.

Note that I rebaselined and re-enabled these two recently:

effect-reference-composite-hw.html 
effect-reference-hw.html 

So it's possible they were just suppressed the last time you ran them.

Please check the results locally if you don't mind, just to see that they look
similar to old-Mesa, since they test a feature that's near to my heart. :)

[Myles C. Maxfield (Amazon.com), 2013-06-19 08:54:51]

On 2013/06/19 01:40:43, Stephen White wrote:
> On 2013/06/19 00:41:31, kbr wrote:
> > On 2013/06/19 00:13:10, Myles C. Maxfield (http://Amazon.com) wrote:
> > > 
> > > Alright, the error is below. It's because Chromium has modified
> > > third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__)
> stanza,
> > > which defines EGLNative*Type. I could make similar modifications to
> > > third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding
> khronos
> > > to the search path before mesa is a better solution.
> > 
> > Gregg Tavares pointed out to me offline that the third_party/khronos headers
> > have been modified to work with Chrome's command buffer implementation, and
> > aren't suitable for use when implementing the actual GL bindings. We have to
> > apply this patch to Mesa's eglplatform.h. Could you please submit a CL doing
> > this, and then resubmit the DEPS roll to the new revision?
> > 
> > > Not only that, but there's another problem: the 'gl' target in
ui/gl/gl.gyp
> > > specifies 'third_party/mesa/src/include' in its 'include_dirs' stanza, but
> > > doesn't actually depend on mesa_headers. This means that the
> > > direct_dependent_settings stanza defined in the 'mesa_headers' target
> doesn't
> > > actually get applied, which results in the "unknown pragma" error that
we've
> > > seen before. If you're interested, I can reproduce this as well.
> > 
> > I don't follow this --
> > http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates that
> the
> > top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
> > understand it your new suppressions in mesa.gyp:mesa_headers for unknown
> pragmas
> > should be taking effect.
> > 
> > > Separate issue: as you can see, almost all of the trybots trying out
> > > https://codereview.chromium.org/17422003/ are red. I started specifically
> > > looking at the mac_gpu_retina bots. In particular, Dirk ran these bots
> twice,
> > > and the same set of 6 tests failed both times. However, on my own retina
> mac,
> > 5
> > > of the 6 tests pass, and the last one fails on ToT as well. In addition, I
> > > believe that the gpu tests should be orthogonal to mesa, since osmesa is,
by
> > > definition, software-based, and doesn't touch the GPU. I believe the GPU
> tests
> > > are testing the system OpenGL implementation, and therefore they shouldn't
> be
> > > changed at all. I was hoping for some guidance about what you think about
> > this,
> > > Ken.
> > 
> > It's definitely possible that the _gpu bot try failures are unrelated to
your
> > patch. zmo knows the state of the gpu try servers better than I do. Right
now
> > we're just trying to get basic coverage of your patch on the majority of our
> > configurations.
> > 
> > Note that there are failures on linux_layout_rel and mac_layout_rel that
> > indicate more suppressions are needed in skia_test_expectations.txt; please
> look
> > through the try job results and apply any changes you think are needed. Feel
> > free to ping us about sending off try jobs against newer patches.
> 
> Note that I rebaselined and re-enabled these two recently:
> 
> effect-reference-composite-hw.html 
> effect-reference-hw.html 
> 
> So it's possible they were just suppressed the last time you ran them.
> 
> Please check the results locally if you don't mind, just to see that they look
> similar to old-Mesa, since they test a feature that's near to my heart. :)

A couple things:

1. effect-reference-composite-hw.html failed on a couple of the trybots
(specifically win_layout_rel, mac_layout_rel, and mac_layout), but I haven't
been able to get them to fail on my local machine. I've added those two to the
skia_test_expectaions.txt document, with a comment stating what's going on

2. I've uploaded a CL which modifies eglplatform.h to include the __APPLE__
stanza that already exists in Chromium's Khronos headers.
https://codereview.chromium.org/17452002/

3. I haven't updated DEPS in this second patchset to point to this modified
eglplatform.h code because I don't know which SVN revision to point it to
(because the eglplatform.h code hasn't been submitted yet)

4. I can't reproduce the unknown pragmas problem that I previously described.
You're right about the mesa_headers dependency; I'm not sure what specifically
was going on, but it seems to be working now.

5. The win, mac_asan, and linux_layout bots' errors seem to be unrelated to the
code I'm working on. I'm going to rerun those bots tomorrow.

6. mac_rel, win_rel, linux_rel, and linux_asan all are ::only:: reporting the
failures due to the missing binary expected .png files. They're good to go.

7. win_gpu and mac_gpu_retina have a set of failing tests inside the
content_browsertests suite. All of these tests are marked as "MANUAL". I don't
know what this means; do you?

8. mac_gpu_retina has some failures that I can't reproduce on my own retina MBP.
zmo: do you know what's going on here?

Thanks,
Myles

[Myles C. Maxfield (Amazon.com), 2013-06-19 08:57:27]

On 2013/06/19 08:54:51, Myles C. Maxfield (Amazon.com) wrote:
> On 2013/06/19 01:40:43, Stephen White wrote:
> > On 2013/06/19 00:41:31, kbr wrote:
> > > On 2013/06/19 00:13:10, Myles C. Maxfield (http://Amazon.com) wrote:
> > > > 
> > > > Alright, the error is below. It's because Chromium has modified
> > > > third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__)
> > stanza,
> > > > which defines EGLNative*Type. I could make similar modifications to
> > > > third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding
> > khronos
> > > > to the search path before mesa is a better solution.
> > > 
> > > Gregg Tavares pointed out to me offline that the third_party/khronos
headers
> > > have been modified to work with Chrome's command buffer implementation,
and
> > > aren't suitable for use when implementing the actual GL bindings. We have
to
> > > apply this patch to Mesa's eglplatform.h. Could you please submit a CL
doing
> > > this, and then resubmit the DEPS roll to the new revision?
> > > 
> > > > Not only that, but there's another problem: the 'gl' target in
> ui/gl/gl.gyp
> > > > specifies 'third_party/mesa/src/include' in its 'include_dirs' stanza,
but
> > > > doesn't actually depend on mesa_headers. This means that the
> > > > direct_dependent_settings stanza defined in the 'mesa_headers' target
> > doesn't
> > > > actually get applied, which results in the "unknown pragma" error that
> we've
> > > > seen before. If you're interested, I can reproduce this as well.
> > > 
> > > I don't follow this --
> > > http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates
that
> > the
> > > top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
> > > understand it your new suppressions in mesa.gyp:mesa_headers for unknown
> > pragmas
> > > should be taking effect.
> > > 
> > > > Separate issue: as you can see, almost all of the trybots trying out
> > > > https://codereview.chromium.org/17422003/ are red. I started
specifically
> > > > looking at the mac_gpu_retina bots. In particular, Dirk ran these bots
> > twice,
> > > > and the same set of 6 tests failed both times. However, on my own retina
> > mac,
> > > 5
> > > > of the 6 tests pass, and the last one fails on ToT as well. In addition,
I
> > > > believe that the gpu tests should be orthogonal to mesa, since osmesa
is,
> by
> > > > definition, software-based, and doesn't touch the GPU. I believe the GPU
> > tests
> > > > are testing the system OpenGL implementation, and therefore they
shouldn't
> > be
> > > > changed at all. I was hoping for some guidance about what you think
about
> > > this,
> > > > Ken.
> > > 
> > > It's definitely possible that the _gpu bot try failures are unrelated to
> your
> > > patch. zmo knows the state of the gpu try servers better than I do. Right
> now
> > > we're just trying to get basic coverage of your patch on the majority of
our
> > > configurations.
> > > 
> > > Note that there are failures on linux_layout_rel and mac_layout_rel that
> > > indicate more suppressions are needed in skia_test_expectations.txt;
please
> > look
> > > through the try job results and apply any changes you think are needed.
Feel
> > > free to ping us about sending off try jobs against newer patches.
> > 
> > Note that I rebaselined and re-enabled these two recently:
> > 
> > effect-reference-composite-hw.html 
> > effect-reference-hw.html 
> > 
> > So it's possible they were just suppressed the last time you ran them.
> > 
> > Please check the results locally if you don't mind, just to see that they
look
> > similar to old-Mesa, since they test a feature that's near to my heart. :)
> 
> A couple things:
> 
> 1. effect-reference-composite-hw.html failed on a couple of the trybots
> (specifically win_layout_rel, mac_layout_rel, and mac_layout), but I haven't
> been able to get them to fail on my local machine. I've added those two to the
> skia_test_expectaions.txt document, with a comment stating what's going on
> 
> 2. I've uploaded a CL which modifies eglplatform.h to include the __APPLE__
> stanza that already exists in Chromium's Khronos headers.
> https://codereview.chromium.org/17452002/
> 
> 3. I haven't updated DEPS in this second patchset to point to this modified
> eglplatform.h code because I don't know which SVN revision to point it to
> (because the eglplatform.h code hasn't been submitted yet)
> 
> 4. I can't reproduce the unknown pragmas problem that I previously described.
> You're right about the mesa_headers dependency; I'm not sure what specifically
> was going on, but it seems to be working now.
> 
> 5. The win, mac_asan, and linux_layout bots' errors seem to be unrelated to
the
> code I'm working on. I'm going to rerun those bots tomorrow.
> 
> 6. mac_rel, win_rel, linux_rel, and linux_asan all are ::only:: reporting the
> failures due to the missing binary expected .png files. They're good to go.
> 
> 7. win_gpu and mac_gpu_retina have a set of failing tests inside the
> content_browsertests suite. All of these tests are marked as "MANUAL". I don't
> know what this means; do you?
> 
> 8. mac_gpu_retina has some failures that I can't reproduce on my own retina
MBP.
> zmo: do you know what's going on here?
> 
> Thanks,
> Myles

9. After two small changes to mesa.gyp, I ran a full linux clang build, and it
was successful. I'll retry this on the trybots tomorrow.

10. dpranke has given me privileges to submit jobs to the try servers, so I'll
do that tomorrow as soon as https://codereview.chromium.org/17452002/ gets
committed.

[Myles C. Maxfield (Amazon.com), 2013-06-19 09:19:56]

Stephen White: I just verified that effect-reference-composite-hw and
effect-reference-hw look essentially the same, and updated
skia_test_expectations.txt. However, effect-reference-hw is ::almost::
exactly the same, but not quite. I wanted to send you the actual and
expected images to see if this difference is okay, or if I should start
digging. I've attached both of them. Please let me know.

Thanks,
Myles C. Maxfield

On 6/18/13 6:40 PM, "senorblanco@chromium.org" <senorblanco@chromium.org>
wrote:

>On 2013/06/19 00:41:31, kbr wrote:
>> On 2013/06/19 00:13:10, Myles C. Maxfield (http://Amazon.com) wrote:
>> >
>> > Alright, the error is below. It's because Chromium has modified
>> > third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__)
>stanza,
>> > which defines EGLNative*Type. I could make similar modifications to
>> > third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding
>khronos
>> > to the search path before mesa is a better solution.
>
>> Gregg Tavares pointed out to me offline that the third_party/khronos
>> headers
>> have been modified to work with Chrome's command buffer implementation,
>> 
>> and
>> aren't suitable for use when implementing the actual GL bindings. We
>>have  
>> to
>> apply this patch to Mesa's eglplatform.h. Could you please submit a CL
>> doing
>> this, and then resubmit the DEPS roll to the new revision?
>
>> > Not only that, but there's another problem: the 'gl' target in
>> ui/gl/gl.gyp
>> > specifies 'third_party/mesa/src/include' in its 'include_dirs'
>>stanza,  
>> but
>> > doesn't actually depend on mesa_headers. This means that the
>> > direct_dependent_settings stanza defined in the 'mesa_headers' target
>doesn't
>> > actually get applied, which results in the "unknown pragma" error
>>that  
>> we've
>> > seen before. If you're interested, I can reproduce this as well.
>
>> I don't follow this --
>> http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates
>> that
>the
>> top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
>> understand it your new suppressions in mesa.gyp:mesa_headers for unknown
>pragmas
>> should be taking effect.
>
>> > Separate issue: as you can see, almost all of the trybots trying out
>> > https://codereview.chromium.org/17422003/ are red. I started
>> specifically
>> > looking at the mac_gpu_retina bots. In particular, Dirk ran these bots
>twice,
>> > and the same set of 6 tests failed both times. However, on my own
>>retina
>mac,
>> 5
>> > of the 6 tests pass, and the last one fails on ToT as well. In
>> addition, I
>> > believe that the gpu tests should be orthogonal to mesa, since osmesa
>> 
>> is, by
>> > definition, software-based, and doesn't touch the GPU. I believe the
>>GPU
>tests
>> > are testing the system OpenGL implementation, and therefore they
>> shouldn't
>be
>> > changed at all. I was hoping for some guidance about what you think
>> about
>> this,
>> > Ken.
>
>> It's definitely possible that the _gpu bot try failures are unrelated
>>to  
>> your
>> patch. zmo knows the state of the gpu try servers better than I do.
>>Right  
>> now
>> we're just trying to get basic coverage of your patch on the majority
>>of  
>> our
>> configurations.
>
>> Note that there are failures on linux_layout_rel and mac_layout_rel that
>> indicate more suppressions are needed in skia_test_expectations.txt;
>> please
>look
>> through the try job results and apply any changes you think are needed.
>> 
>> Feel
>> free to ping us about sending off try jobs against newer patches.
>
>Note that I rebaselined and re-enabled these two recently:
>
>effect-reference-composite-hw.html
>effect-reference-hw.html
>
>So it's possible they were just suppressed the last time you ran them.
>
>Please check the results locally if you don't mind, just to see that they
> 
>look
>similar to old-Mesa, since they test a feature that's near to my heart. :)
>
>https://chromiumcodereview.appspot.com/17005007/

[Stephen White, 2013-06-19 11:21:25]

+sugoi

Hmm, interesting. It looks like the turbulence result has fewer octaves of
noise in the newer version of Mesa. Alexis, does this look ok to you?

Either way, I don't think this should be a blocker, since everything else
looks fine and we have "real GPU" test coverage of this algorithm in Skia.

Thanks for the extra effort,

Stephen


On Wed, Jun 19, 2013 at 5:19 AM, Maxfield, Myles <mymax@amazon.com> wrote:

> Stephen White: I just verified that effect-reference-composite-hw and
> effect-reference-hw look essentially the same, and updated
> skia_test_expectations.txt. However, effect-reference-hw is ::almost::
> exactly the same, but not quite. I wanted to send you the actual and
> expected images to see if this difference is okay, or if I should start
> digging. I've attached both of them. Please let me know.
>
> Thanks,
> Myles C. Maxfield
>
> On 6/18/13 6:40 PM, "senorblanco@chromium.org" <senorblanco@chromium.org>
> wrote:
>
> >On 2013/06/19 00:41:31, kbr wrote:
> >> On 2013/06/19 00:13:10, Myles C. Maxfield (http://Amazon.com) wrote:
> >> >
> >> > Alright, the error is below. It's because Chromium has modified
> >> > third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__)
> >stanza,
> >> > which defines EGLNative*Type. I could make similar modifications to
> >> > third_party/mesa/src/include/EGL/eglplatform.h, but I feel that adding
> >khronos
> >> > to the search path before mesa is a better solution.
> >
> >> Gregg Tavares pointed out to me offline that the third_party/khronos
> >> headers
> >> have been modified to work with Chrome's command buffer implementation,
> >>
> >> and
> >> aren't suitable for use when implementing the actual GL bindings. We
> >>have
> >> to
> >> apply this patch to Mesa's eglplatform.h. Could you please submit a CL
> >> doing
> >> this, and then resubmit the DEPS roll to the new revision?
> >
> >> > Not only that, but there's another problem: the 'gl' target in
> >> ui/gl/gl.gyp
> >> > specifies 'third_party/mesa/src/include' in its 'include_dirs'
> >>stanza,
> >> but
> >> > doesn't actually depend on mesa_headers. This means that the
> >> > direct_dependent_settings stanza defined in the 'mesa_headers' target
> >doesn't
> >> > actually get applied, which results in the "unknown pragma" error
> >>that
> >> we've
> >> > seen before. If you're interested, I can reproduce this as well.
> >
> >> I don't follow this --
> >> http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates
> >> that
> >the
> >> top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
> >> understand it your new suppressions in mesa.gyp:mesa_headers for unknown
> >pragmas
> >> should be taking effect.
> >
> >> > Separate issue: as you can see, almost all of the trybots trying out
> >> > https://codereview.chromium.org/17422003/ are red. I started
> >> specifically
> >> > looking at the mac_gpu_retina bots. In particular, Dirk ran these bots
> >twice,
> >> > and the same set of 6 tests failed both times. However, on my own
> >>retina
> >mac,
> >> 5
> >> > of the 6 tests pass, and the last one fails on ToT as well. In
> >> addition, I
> >> > believe that the gpu tests should be orthogonal to mesa, since osmesa
> >>
> >> is, by
> >> > definition, software-based, and doesn't touch the GPU. I believe the
> >>GPU
> >tests
> >> > are testing the system OpenGL implementation, and therefore they
> >> shouldn't
> >be
> >> > changed at all. I was hoping for some guidance about what you think
> >> about
> >> this,
> >> > Ken.
> >
> >> It's definitely possible that the _gpu bot try failures are unrelated
> >>to
> >> your
> >> patch. zmo knows the state of the gpu try servers better than I do.
> >>Right
> >> now
> >> we're just trying to get basic coverage of your patch on the majority
> >>of
> >> our
> >> configurations.
> >
> >> Note that there are failures on linux_layout_rel and mac_layout_rel that
> >> indicate more suppressions are needed in skia_test_expectations.txt;
> >> please
> >look
> >> through the try job results and apply any changes you think are needed.
> >>
> >> Feel
> >> free to ping us about sending off try jobs against newer patches.
> >
> >Note that I rebaselined and re-enabled these two recently:
> >
> >effect-reference-composite-hw.html
> >effect-reference-hw.html
> >
> >So it's possible they were just suppressed the last time you ran them.
> >
> >Please check the results locally if you don't mind, just to see that they
> >
> >look
> >similar to old-Mesa, since they test a feature that's near to my heart. :)
> >
> >https://chromiumcodereview.appspot.com/17005007/
>
>

[sugoi, 2013-06-19 12:14:32]

I can look into it once this is merged. I'm not even sure that the current
version of MESA does a good job with the Perlin noise, so I wouldn't worry
about it too much.


On Wed, Jun 19, 2013 at 7:21 AM, Stephen White <senorblanco@chromium.org>wrote:

> +sugoi
>
> Hmm, interesting. It looks like the turbulence result has fewer octaves of
> noise in the newer version of Mesa. Alexis, does this look ok to you?
>
> Either way, I don't think this should be a blocker, since everything else
> looks fine and we have "real GPU" test coverage of this algorithm in Skia.
>
> Thanks for the extra effort,
>
> Stephen
>
>
> On Wed, Jun 19, 2013 at 5:19 AM, Maxfield, Myles <mymax@amazon.com> wrote:
>
>> Stephen White: I just verified that effect-reference-composite-hw and
>> effect-reference-hw look essentially the same, and updated
>> skia_test_expectations.txt. However, effect-reference-hw is ::almost::
>> exactly the same, but not quite. I wanted to send you the actual and
>> expected images to see if this difference is okay, or if I should start
>> digging. I've attached both of them. Please let me know.
>>
>> Thanks,
>> Myles C. Maxfield
>>
>> On 6/18/13 6:40 PM, "senorblanco@chromium.org" <senorblanco@chromium.org>
>> wrote:
>>
>> >On 2013/06/19 00:41:31, kbr wrote:
>> >> On 2013/06/19 00:13:10, Myles C. Maxfield (http://Amazon.com) wrote:
>> >> >
>> >> > Alright, the error is below. It's because Chromium has modified
>> >> > third_party/khronos/EGL/eglplatform.h to include a defined(__APPLE__)
>> >stanza,
>> >> > which defines EGLNative*Type. I could make similar modifications to
>> >> > third_party/mesa/src/include/EGL/eglplatform.h, but I feel that
>> adding
>> >khronos
>> >> > to the search path before mesa is a better solution.
>> >
>> >> Gregg Tavares pointed out to me offline that the third_party/khronos
>> >> headers
>> >> have been modified to work with Chrome's command buffer implementation,
>> >>
>> >> and
>> >> aren't suitable for use when implementing the actual GL bindings. We
>> >>have
>> >> to
>> >> apply this patch to Mesa's eglplatform.h. Could you please submit a CL
>> >> doing
>> >> this, and then resubmit the DEPS roll to the new revision?
>> >
>> >> > Not only that, but there's another problem: the 'gl' target in
>> >> ui/gl/gl.gyp
>> >> > specifies 'third_party/mesa/src/include' in its 'include_dirs'
>> >>stanza,
>> >> but
>> >> > doesn't actually depend on mesa_headers. This means that the
>> >> > direct_dependent_settings stanza defined in the 'mesa_headers' target
>> >doesn't
>> >> > actually get applied, which results in the "unknown pragma" error
>> >>that
>> >> we've
>> >> > seen before. If you're interested, I can reproduce this as well.
>> >
>> >> I don't follow this --
>> >> http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp indicates
>> >> that
>> >the
>> >> top of tree gl.gyp does depend on mesa.gyp:mesa_headers, so as far as I
>> >> understand it your new suppressions in mesa.gyp:mesa_headers for
>> unknown
>> >pragmas
>> >> should be taking effect.
>> >
>> >> > Separate issue: as you can see, almost all of the trybots trying out
>> >> > https://codereview.chromium.org/17422003/ are red. I started
>> >> specifically
>> >> > looking at the mac_gpu_retina bots. In particular, Dirk ran these
>> bots
>> >twice,
>> >> > and the same set of 6 tests failed both times. However, on my own
>> >>retina
>> >mac,
>> >> 5
>> >> > of the 6 tests pass, and the last one fails on ToT as well. In
>> >> addition, I
>> >> > believe that the gpu tests should be orthogonal to mesa, since osmesa
>> >>
>> >> is, by
>> >> > definition, software-based, and doesn't touch the GPU. I believe the
>> >>GPU
>> >tests
>> >> > are testing the system OpenGL implementation, and therefore they
>> >> shouldn't
>> >be
>> >> > changed at all. I was hoping for some guidance about what you think
>> >> about
>> >> this,
>> >> > Ken.
>> >
>> >> It's definitely possible that the _gpu bot try failures are unrelated
>> >>to
>> >> your
>> >> patch. zmo knows the state of the gpu try servers better than I do.
>> >>Right
>> >> now
>> >> we're just trying to get basic coverage of your patch on the majority
>> >>of
>> >> our
>> >> configurations.
>> >
>> >> Note that there are failures on linux_layout_rel and mac_layout_rel
>> that
>> >> indicate more suppressions are needed in skia_test_expectations.txt;
>> >> please
>> >look
>> >> through the try job results and apply any changes you think are needed.
>> >>
>> >> Feel
>> >> free to ping us about sending off try jobs against newer patches.
>> >
>> >Note that I rebaselined and re-enabled these two recently:
>> >
>> >effect-reference-composite-hw.html
>> >effect-reference-hw.html
>> >
>> >So it's possible they were just suppressed the last time you ran them.
>> >
>> >Please check the results locally if you don't mind, just to see that they
>> >
>> >look
>> >similar to old-Mesa, since they test a feature that's near to my heart.
>> :)
>> >
>> >https://chromiumcodereview.appspot.com/17005007/
>>
>>
>

[apatrick_chromium, 2013-06-21 00:31:42]

lgtm

[nduca, 2013-06-21 00:37:02]

lucky you lgtm

[Dirk Pranke, 2013-07-02 01:46:05]

Committed patchset #14 manually as r209579.