Changes to support Angle on WinRT devices (Windows Store Apps)

Changes to support Angle on WinRT devices (Windows Store Apps)

WinRT (Windows Store Apps) support was added to Angle. Support for
building Win32 and ARM versions. Support for DirectX Level 9_1 for
Surface RT up Level 11_1 was added. Compiling HLSL for DirectX Level 9_1.

[Shannon Woods, 2013-08-27 19:26:14]

This changelist is very large. It might be easier to review, and to understand
the design and direction of your work, if it were broken into more incremental
chunks that could be independently reviewed. Is that a possibility?

Some notes on the overall work-- 

We can't add functions to EGL that aren't described in the specification, except
through the extension process, as it's a standard API. The specification for EGL
1.4 can be found here:
http://www.khronos.org/registry/egl/specs/eglspec.1.4.20130211.pdf

And an overview of Khronos's extension process is here:
http://www.opengl.org/registry/doc/rules.html

That said, before creating an extension, we have to consider whether there is
significant need for the extension, and whether the functionality is already
provided by another part of the specification. Essentially, the barrier to
adding functionality to the EGL API, even as an extension, is considerably
higher than the barrier to adding our own internal functions.

I also noted that there appear to be several places in the diff that look like
they remove recently added changes to ANGLE. Is this because the work was
branched at an earlier point, and a diff generated from the branch against
master? It's important not to lose these changes if this work lands.

On the new solution files-- is it necessary to have one solution file for WinRT
support, and another for WinRT support + Visual Studio 2013 support? Or can one
WinRT solution be compatible with VS2010+? If it's possible, we'd like to have
as few proliferating solutions and project files as possible. Ideally it would
be nice to have a single solution, which could build DX9 support in or out
conditionally, allowing it to be excluded for WinRT builds-- but I'm not a
Visual Studio configuration expert, so I'm not sure of the feasibility of this.
Others may have more insightful commentary on this point.

There is some code included in the changelist which bears a Microsoft copyright.
What is this code's provenance, and what are its licensing terms? As an open
source project, we must be diligent about ensuring that we accept only code we
have the right to use.

On SwiftShader-- there's a todo asking what this .dll does. SwiftShader is a
software renderer licensed by Chrome. It's probably safe to assume for now that
it won't be used with WinRT.

And finally, on feature levels below 10_0-- I haven't yet completely reviewed
this code, but have you implemented workarounds for features which do not exist
below that level, or just prevented those features from being used on lower
levels? For example, in order to support point sprites on DirectX 11, we found
it necessary to emulate them via geometry shaders. If an application attempts to
use point sprites in WinRT on a device which does not support feature level
10_0, will they work?

[stammen, 2013-08-27 21:23:37]

Hi Shannon,

Thank you for your feedback. Here are the answers to your questions


On Tue, Aug 27, 2013 at 12:26 PM, <shannonwoods@chromium.org> wrote:

> This changelist is very large. It might be easier to review, and to
> understand the design and direction of your work, if it were broken into
> more incremental chunks that could be independently reviewed. Is that a
> possibility?
>

We tried to make as few changes as possible. I put a slightly updated
public version of the patch at

https://github.com/stammen/angleproject/tree/winrt-patch-1.2.0.2438

We will discuss this and get back to you.



>
> Some notes on the overall work--
>
> We can't add functions to EGL that aren't described in the
> specification, except through the extension process, as it's a standard
> API. The specification for EGL 1.4 can be found here:
>
http://www.khronos.org/**registry/egl/specs/eglspec.1.**4.20130211.pdf<http:/...


>
> And an overview of Khronos's extension process is here:
>
http://www.opengl.org/**registry/doc/rules.html<http://www.opengl.org/registr...
>
> That said, before creating an extension, we have to consider whether
> there is significant need for the extension, and whether the
> functionality is already provided by another part of the specification.
> Essentially, the barrier to adding functionality to the EGL API, even as
> an extension, is considerably higher than the barrier to adding our own
> internal functions.
>

We will investigate another solution to avoid changing the  EGL interface.


>
> I also noted that there appear to be several places in the diff that
> look like they remove recently added changes to ANGLE. Is this because
> the work was branched at an earlier point, and a diff generated from the
> branch against master? It's important not to lose these changes if this
> work lands.
>

This work was based on version 1.2.0.2438. Your version was at 2441. We can
update the patch to be off of 1.2.0.2441 (or whatever the current version
Angle is currently).



> On the new solution files-- is it necessary to have one solution file
> for WinRT support, and another for WinRT support + Visual Studio 2013
> support? Or can one WinRT solution be compatible with VS2010+? If it's
> possible, we'd like to have as few proliferating solutions and project
> files as possible. Ideally it would be nice to have a single solution,
> which could build DX9 support in or out conditionally, allowing it to be
> excluded for WinRT builds-- but I'm not a Visual Studio configuration
> expert, so I'm not sure of the feasibility of this. Others may have more
> insightful commentary on this point.
>

Visual Studio is in a state of transition. VS2013 projects are not
compatible with VS2012. WinRT projects are not compatible with VS2010
projects. It will get worse once we add the VS2012 and VS2013 Windows Phone
8.0 projects which are not compatible with the WinRT projects. Take a look
at the DirectXTK project  after (which we sort of modeled our project
structure) https://directxtk.codeplex.com/SourceControl/latest#Readme.txt.
and you will see the problem we will have to support all of the various
Visual Studio project/device versions. We are open to suggestions on how to
fix this complex project issue.


> There is some code included in the changelist which bears a Microsoft
> copyright. What is this code's provenance, and what are its licensing
> terms? As an open source project, we must be diligent about ensuring
> that we accept only code we have the right to use.
>

This code is under the Microsoft permissive license which I have attached
for your review. Where should we include this license in the Angle code
base?


>
> On SwiftShader-- there's a todo asking what this .dll does. SwiftShader
> is a software renderer licensed by Chrome. It's probably safe to assume
> for now that it won't be used with WinRT.
>

Thanks for the update.


>
> And finally, on feature levels below 10_0-- I haven't yet completely
> reviewed this code, but have you implemented workarounds for features
> which do not exist below that level, or just prevented those features
> from being used on lower levels? For example, in order to support point
> sprites on DirectX 11, we found it necessary to emulate them via
> geometry shaders. If an application attempts to use point sprites in
> WinRT on a device which does not support feature level 10_0, will they
> work?
>

We have implemented feature level9_1 workarounds for most of the features
that do not exist. However, it appears we forgot to do point sprites. We
need to add some code to draw a textured quad the size of whatever the
current point size is set to. This will be in the next version. The biggest
issue is that level 9_1 shaders only support a limited number of
instructions so some shaders will not compile.

Would you like us to fix these issues and resubmit? What is the procedure
for moving forward?

Thanks,

Dale


>
>
https://codereview.appspot.**com/12917046/<https://codereview.appspot.com/129...
>

[Shannon Woods, 2013-09-03 18:53:41]

> Visual Studio is in a state of transition. VS2013 projects are not
> compatible with VS2012. WinRT projects are not compatible with VS2010
> projects. It will get worse once we add the VS2012 and VS2013 Windows Phone
> 8.0 projects which are not compatible with the WinRT projects. Take a look
> at the DirectXTK project  after (which we sort of modeled our project
> structure) https://directxtk.codeplex.com/SourceControl/latest#Readme.txt.
> and you will see the problem we will have to support all of the various
> Visual Studio project/device versions. We are open to suggestions on how to
> fix this complex project issue.

It's definitely a complicated issue to tackle. One of the reasons I'd like
to avoid spawning many different Visual Studio projects is that I fear it 
will become difficult to keep them all in sync should we start making build
changes that affect all the separate projects, and keeping all of them in 
sync with changes made to the gyp build files, which chromium uses to build
ANGLE.

It would be helpful if we could consider the build management issue separately
from the WinRT support issue, as much as possible. To that end, could we look
at just the build files needed to support WinRT on one version of Visual Studio
for now? (2012, for preference, as it's already released.)

> > There is some code included in the changelist which bears a Microsoft
> > copyright. What is this code's provenance, and what are its licensing
> > terms? As an open source project, we must be diligent about ensuring
> > that we accept only code we have the right to use.
> >
> 
> This code is under the Microsoft permissive license which I have attached
> for your review. Where should we include this license in the Angle code
> base?
 
Any code not authored by ANGLE project contributors should go in 
src/third_party, along with its license. See the items included there 
currently for examples of how we document the license.

> Would you like us to fix these issues and resubmit? What is the procedure
> for moving forward?

Ideally, yes. If it's possible to winnow this down into more incremental
patches, then they should be submitted as individual code reviews-- 
although from the linked project pages, it looks as though the work was all
done in a single monolithic commit, for the most part.

From the broad view, here are the items that it would be good to consider 
separately from one another:

  * Build system changes needed to bring in WinRT dependencies and omit D3D9 
    dependencies from WinRT builds
  * Changes primarily to the EGL layer to handle the differences in standard 
    Windows vs. WinRT window handling and threading models.
  * Changes to the GL layer to handle feature levels below 10_0.

One of the large reasons I'd like to be able to look at these separately--
aside from being able to give feedback before processing all several hundred 
lines of changes-- is that WinRT support is not necessarily dependent on all 
of these changes going through. Support for WinRT on feature level 10_0 devices,
for example, does not depend on the changes for feature level 9_1 support, so
it makes sense to consider them separately.

[stammen, 2013-09-05 15:46:25]

Hi Shannon,

We will make the requested changes and resubmit in smaller patches as you
have suggested.

We will provide only VS2012 projects. If a dev wants to use VS2013 he or
she can use VS2013 to convert the project files to VS2013 format.

We are almost finished with Windows Phone 8.0 support and Windows 8.0 WinRT
support (using precompiled shaders). We will need to provide
separate VS2012 project files for Windows Phone 8. We won't submit the
Windows Phone changes until review of WinRT version is complete.

Thanks,

Dale




On Tue, Sep 3, 2013 at 11:53 AM, <shannonwoods@chromium.org> wrote:

> Visual Studio is in a state of transition. VS2013 projects are not
>> compatible with VS2012. WinRT projects are not compatible with VS2010
>> projects. It will get worse once we add the VS2012 and VS2013 Windows
>>
> Phone
>
>> 8.0 projects which are not compatible with the WinRT projects. Take a
>>
> look
>
>> at the DirectXTK project  after (which we sort of modeled our project
>> structure)
>>
>
https://directxtk.codeplex.**com/SourceControl/latest#**Readme.txt<https://di...
> .
>
>> and you will see the problem we will have to support all of the
>>
> various
>
>> Visual Studio project/device versions. We are open to suggestions on
>>
> how to
>
>> fix this complex project issue.
>>
>
> It's definitely a complicated issue to tackle. One of the reasons I'd
> like
> to avoid spawning many different Visual Studio projects is that I fear
> it
> will become difficult to keep them all in sync should we start making
> build
> changes that affect all the separate projects, and keeping all of them
> in
> sync with changes made to the gyp build files, which chromium uses to
> build
> ANGLE.
>
> It would be helpful if we could consider the build management issue
> separately
> from the WinRT support issue, as much as possible. To that end, could we
> look
> at just the build files needed to support WinRT on one version of Visual
> Studio
> for now? (2012, for preference, as it's already released.)
>
>
>  > There is some code included in the changelist which bears a
>>
> Microsoft
>
>> > copyright. What is this code's provenance, and what are its
>>
> licensing
>
>> > terms? As an open source project, we must be diligent about ensuring
>> > that we accept only code we have the right to use.
>> >
>>
>
>  This code is under the Microsoft permissive license which I have
>>
> attached
>
>> for your review. Where should we include this license in the Angle
>>
> code
>
>> base?
>>
>
> Any code not authored by ANGLE project contributors should go in
> src/third_party, along with its license. See the items included there
> currently for examples of how we document the license.
>
>
>  Would you like us to fix these issues and resubmit? What is the
>>
> procedure
>
>> for moving forward?
>>
>
> Ideally, yes. If it's possible to winnow this down into more incremental
> patches, then they should be submitted as individual code reviews--
> although from the linked project pages, it looks as though the work was
> all
> done in a single monolithic commit, for the most part.
>
> From the broad view, here are the items that it would be good to
> consider
> separately from one another:
>
>   * Build system changes needed to bring in WinRT dependencies and omit
> D3D9
>     dependencies from WinRT builds
>   * Changes primarily to the EGL layer to handle the differences in
> standard
>     Windows vs. WinRT window handling and threading models.
>   * Changes to the GL layer to handle feature levels below 10_0.
>
> One of the large reasons I'd like to be able to look at these
> separately--
> aside from being able to give feedback before processing all several
> hundred
> lines of changes-- is that WinRT support is not necessarily dependent on
> all
> of these changes going through. Support for WinRT on feature level 10_0
> devices,
> for example, does not depend on the changes for feature level 9_1
> support, so
> it makes sense to consider them separately.
>
>
https://codereview.appspot.**com/12917046/<https://codereview.appspot.com/129...
>

[shannonwoods1, 2013-09-06 15:30:56]

Fantastic, thank you!

One caveat about 8.0 Phone & RT support-- ANGLE is intended to be a
conformant implementation of OpenGL ES 2.0, and the GL ES spec requires
run-time shader compilation. We may not be able to accept changes which
introduce precompiled shader workarounds, and remove support for run-time
compilation. Anything which would intentionally break conformance would be
better suited for a platform-specific fork or separate project.


On Thu, Sep 5, 2013 at 11:45 AM, Dale Stammen <stammen@gmail.com> wrote:

> Hi Shannon,
>
> We will make the requested changes and resubmit in smaller patches as you
> have suggested.
>
> We will provide only VS2012 projects. If a dev wants to use VS2013 he or
> she can use VS2013 to convert the project files to VS2013 format.
>
> We are almost finished with Windows Phone 8.0 support and Windows 8.0
> WinRT support (using precompiled shaders). We will need to provide
> separate VS2012 project files for Windows Phone 8. We won't submit the
> Windows Phone changes until review of WinRT version is complete.
>
> Thanks,
>
> Dale
>
>
>
>
> On Tue, Sep 3, 2013 at 11:53 AM, <shannonwoods@chromium.org> wrote:
>
>> Visual Studio is in a state of transition. VS2013 projects are not
>>> compatible with VS2012. WinRT projects are not compatible with VS2010
>>> projects. It will get worse once we add the VS2012 and VS2013 Windows
>>>
>> Phone
>>
>>> 8.0 projects which are not compatible with the WinRT projects. Take a
>>>
>> look
>>
>>> at the DirectXTK project  after (which we sort of modeled our project
>>> structure)
>>>
>>
https://directxtk.codeplex.**com/SourceControl/latest#**Readme.txt<https://di...
>> .
>>
>>> and you will see the problem we will have to support all of the
>>>
>> various
>>
>>> Visual Studio project/device versions. We are open to suggestions on
>>>
>> how to
>>
>>> fix this complex project issue.
>>>
>>
>> It's definitely a complicated issue to tackle. One of the reasons I'd
>> like
>> to avoid spawning many different Visual Studio projects is that I fear
>> it
>> will become difficult to keep them all in sync should we start making
>> build
>> changes that affect all the separate projects, and keeping all of them
>> in
>> sync with changes made to the gyp build files, which chromium uses to
>> build
>> ANGLE.
>>
>> It would be helpful if we could consider the build management issue
>> separately
>> from the WinRT support issue, as much as possible. To that end, could we
>> look
>> at just the build files needed to support WinRT on one version of Visual
>> Studio
>> for now? (2012, for preference, as it's already released.)
>>
>>
>>  > There is some code included in the changelist which bears a
>>>
>> Microsoft
>>
>>> > copyright. What is this code's provenance, and what are its
>>>
>> licensing
>>
>>> > terms? As an open source project, we must be diligent about ensuring
>>> > that we accept only code we have the right to use.
>>> >
>>>
>>
>>  This code is under the Microsoft permissive license which I have
>>>
>> attached
>>
>>> for your review. Where should we include this license in the Angle
>>>
>> code
>>
>>> base?
>>>
>>
>> Any code not authored by ANGLE project contributors should go in
>> src/third_party, along with its license. See the items included there
>> currently for examples of how we document the license.
>>
>>
>>  Would you like us to fix these issues and resubmit? What is the
>>>
>> procedure
>>
>>> for moving forward?
>>>
>>
>> Ideally, yes. If it's possible to winnow this down into more incremental
>> patches, then they should be submitted as individual code reviews--
>> although from the linked project pages, it looks as though the work was
>> all
>> done in a single monolithic commit, for the most part.
>>
>> From the broad view, here are the items that it would be good to
>> consider
>> separately from one another:
>>
>>   * Build system changes needed to bring in WinRT dependencies and omit
>> D3D9
>>     dependencies from WinRT builds
>>   * Changes primarily to the EGL layer to handle the differences in
>> standard
>>     Windows vs. WinRT window handling and threading models.
>>   * Changes to the GL layer to handle feature levels below 10_0.
>>
>> One of the large reasons I'd like to be able to look at these
>> separately--
>> aside from being able to give feedback before processing all several
>> hundred
>> lines of changes-- is that WinRT support is not necessarily dependent on
>> all
>> of these changes going through. Support for WinRT on feature level 10_0
>> devices,
>> for example, does not depend on the changes for feature level 9_1
>> support, so
>> it makes sense to consider them separately.
>>
>>
https://codereview.appspot.**com/12917046/<https://codereview.appspot.com/129...
>>
>
>

[stammen, 2013-09-06 17:29:07]

The only change we needed to make is to allow the creation of the renderer
without the successful loading of the d3dcompiler dll since it is not
available for Windows Phone 8.0 and WinRT 8.0. These platforms cannot be
fully OpenGL ES 2.0 compliant due to the lack of the run-time compiler.

The dev then uses (assuming the shader program was already precompiled
using our precompile tool)

glProgramBinaryOES(m_colorProgram, GL_PROGRAM_BINARY_ANGLE, buffer,
bufferSize)


to load the precompiled shader.

Will this not be acceptable? No changes were made to the current API.

Thanks,

Dale



On Fri, Sep 6, 2013 at 8:30 AM, Shannon Woods <shannonwoods@google.com>wrote:

> Fantastic, thank you!
>
> One caveat about 8.0 Phone & RT support-- ANGLE is intended to be a
> conformant implementation of OpenGL ES 2.0, and the GL ES spec requires
> run-time shader compilation. We may not be able to accept changes which
> introduce precompiled shader workarounds, and remove support for run-time
> compilation. Anything which would intentionally break conformance would be
> better suited for a platform-specific fork or separate project.
>
>
> On Thu, Sep 5, 2013 at 11:45 AM, Dale Stammen <stammen@gmail.com> wrote:
>
>> Hi Shannon,
>>
>> We will make the requested changes and resubmit in smaller patches as you
>> have suggested.
>>
>> We will provide only VS2012 projects. If a dev wants to use VS2013 he or
>> she can use VS2013 to convert the project files to VS2013 format.
>>
>> We are almost finished with Windows Phone 8.0 support and Windows 8.0
>> WinRT support (using precompiled shaders). We will need to provide
>> separate VS2012 project files for Windows Phone 8. We won't submit the
>> Windows Phone changes until review of WinRT version is complete.
>>
>> Thanks,
>>
>> Dale
>>
>>
>>
>>
>> On Tue, Sep 3, 2013 at 11:53 AM, <shannonwoods@chromium.org> wrote:
>>
>>> Visual Studio is in a state of transition. VS2013 projects are not
>>>> compatible with VS2012. WinRT projects are not compatible with VS2010
>>>> projects. It will get worse once we add the VS2012 and VS2013 Windows
>>>>
>>> Phone
>>>
>>>> 8.0 projects which are not compatible with the WinRT projects. Take a
>>>>
>>> look
>>>
>>>> at the DirectXTK project  after (which we sort of modeled our project
>>>> structure)
>>>>
>>>
https://directxtk.codeplex.**com/SourceControl/latest#**Readme.txt<https://di...
>>> .
>>>
>>>> and you will see the problem we will have to support all of the
>>>>
>>> various
>>>
>>>> Visual Studio project/device versions. We are open to suggestions on
>>>>
>>> how to
>>>
>>>> fix this complex project issue.
>>>>
>>>
>>> It's definitely a complicated issue to tackle. One of the reasons I'd
>>> like
>>> to avoid spawning many different Visual Studio projects is that I fear
>>> it
>>> will become difficult to keep them all in sync should we start making
>>> build
>>> changes that affect all the separate projects, and keeping all of them
>>> in
>>> sync with changes made to the gyp build files, which chromium uses to
>>> build
>>> ANGLE.
>>>
>>> It would be helpful if we could consider the build management issue
>>> separately
>>> from the WinRT support issue, as much as possible. To that end, could we
>>> look
>>> at just the build files needed to support WinRT on one version of Visual
>>> Studio
>>> for now? (2012, for preference, as it's already released.)
>>>
>>>
>>>  > There is some code included in the changelist which bears a
>>>>
>>> Microsoft
>>>
>>>> > copyright. What is this code's provenance, and what are its
>>>>
>>> licensing
>>>
>>>> > terms? As an open source project, we must be diligent about ensuring
>>>> > that we accept only code we have the right to use.
>>>> >
>>>>
>>>
>>>  This code is under the Microsoft permissive license which I have
>>>>
>>> attached
>>>
>>>> for your review. Where should we include this license in the Angle
>>>>
>>> code
>>>
>>>> base?
>>>>
>>>
>>> Any code not authored by ANGLE project contributors should go in
>>> src/third_party, along with its license. See the items included there
>>> currently for examples of how we document the license.
>>>
>>>
>>>  Would you like us to fix these issues and resubmit? What is the
>>>>
>>> procedure
>>>
>>>> for moving forward?
>>>>
>>>
>>> Ideally, yes. If it's possible to winnow this down into more incremental
>>> patches, then they should be submitted as individual code reviews--
>>> although from the linked project pages, it looks as though the work was
>>> all
>>> done in a single monolithic commit, for the most part.
>>>
>>> From the broad view, here are the items that it would be good to
>>> consider
>>> separately from one another:
>>>
>>>   * Build system changes needed to bring in WinRT dependencies and omit
>>> D3D9
>>>     dependencies from WinRT builds
>>>   * Changes primarily to the EGL layer to handle the differences in
>>> standard
>>>     Windows vs. WinRT window handling and threading models.
>>>   * Changes to the GL layer to handle feature levels below 10_0.
>>>
>>> One of the large reasons I'd like to be able to look at these
>>> separately--
>>> aside from being able to give feedback before processing all several
>>> hundred
>>> lines of changes-- is that WinRT support is not necessarily dependent on
>>> all
>>> of these changes going through. Support for WinRT on feature level 10_0
>>> devices,
>>> for example, does not depend on the changes for feature level 9_1
>>> support, so
>>> it makes sense to consider them separately.
>>>
>>>
https://codereview.appspot.**com/12917046/<https://codereview.appspot.com/129...
>>>
>>
>>
>

[aknight, 2013-09-11 21:04:43]

Cool, I just came across this.

We have been experimenting similar changes to ANGLE for use in Qt, and we'd be
happy to help/upstream as well. We do fall into that "third-party fork" category
where compliance is less of a concern, but the fewer patches we need to carry,
the better.

See:
Patch ANGLE to support WinRT
https://codereview.qt-project.org/#change,51858
Implement OpenGL ES 2 support for WinRT QPA
https://codereview.qt-project.org/#change,51857
(WIP) ANGLE: Enable D3D11 for feature level 9 cards
https://codereview.qt-project.org/#change,64933

[aknight, 2013-09-12 10:04:18]

Just a few comments on the EGL implementation.

I strongly feel we should steer clear of using CX extensions in the ANGLE
codebase, as it makes it more difficult to cross ABI boundaries (making it
difficult to use the lib with apps compiled without the ZW switch), and it
requires additional exception wrapping.

It should be of minimal burden to apps which use CX to obtain the C++ interface
pointer when dealing with the native EGL types, so I don't think there is a
compelling argument not to do this.

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h
File include/EGL/eglplatform.h (right):

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:76: #if defined(ANGLE_PLATFORM_WINRT)
You should use a more appropriate condition here (e.g. something based on
WINAPIFAMILY). eglplatform.h is a public, cross-platform header after all.

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:85: } EGLNativeWindowType;
I would prefer that the standard C++ pointer be used here for ICoreWindow, and
that it be the only native handle (no struct). That keeps things compatible with
C/C++ (non-CX) applications, and doesn't take anything away from CX apps (they
can still provide and use the pointer within ANGLE via standard C++). It also
ensures you can build the lib without /ZW.

Having a struct as the window type is questionable because you can't easily cast
the pointer, and you don't always (perhaps often than not) use a
swapchainbackground panel anyway. I'm not sure that the swapchainbackgroundpanel
pointer is really needed, and it causes additional issues when you consider that
Windows 8.1 adds new XAML swap chain panel types as well.

This would also reduce the need to include those headers (if you forward declare
the pointer types).

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:87: typedef int EGLNativeDisplayType;
Since Windows 8.1 introduces multiple view support, this should probably be a
pointer to ICoreApplicationView (even if it isn't yet used). While
EGL_DEFAULT_DISPLAY will be the most common use, the ability to create new views
in the application suggests that we need a way for EGL to track new "displays".

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:94: #endif // ANGLE_PLATFORM_WINRT
The WINRT defs should go in a separate block instead of inside the Windows
block, IMO.

[aknight, 2013-09-12 10:24:20]

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h
File include/EGL/eglplatform.h (right):

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:86: 
Looking at this again, EGLNativeWindowType should really be an IUnknown pointer.
That way, ANGLE can QueryInterface() for either ICoreWindow or
ISwapChainBackgroundPanelNative, and make the right swap chain choice
internally. This also means that future D3D swap chain devices can be handled
without modifying this header.

[Shannon Woods, 2013-09-12 14:01:50]

On 2013/09/06 17:29:07, stammen wrote:
> The only change we needed to make is to allow the creation of the renderer
> without the successful loading of the d3dcompiler dll since it is not
> available for Windows Phone 8.0 and WinRT 8.0. These platforms cannot be
> fully OpenGL ES 2.0 compliant due to the lack of the run-time compiler.
> 
> The dev then uses (assuming the shader program was already precompiled
> using our precompile tool)
> 
> glProgramBinaryOES(m_colorProgram, GL_PROGRAM_BINARY_ANGLE, buffer,
> bufferSize)
> 
> 
> to load the precompiled shader.
> 
> Will this not be acceptable? No changes were made to the current API.
> 
> Thanks,
> 
> Dale

What would happen if the application attempted to call
glCompileShader/glLinkProgram on Windows Phone or WinRT 8.0? In order to
maintain compliance, we need not only to avoid adding functionality to the API
that is not there, but we also must support all API entry points as described in
the spec. We can't dictate to developers that they must load shaders only via
glProgramBinaryOES, because the spec still contains the on-demand compile path.
(On the other hand, we'd technically be free to drop glProgramBinaryOES, as it's
added by an extension, so its support is optional.)

[Shannon Woods, 2013-09-12 14:25:28]

@aknight - Thank you very much for your comments. Input from developers who have
investigated adding similar support is welcome and appreciated.

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h
File include/EGL/eglplatform.h (right):

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:85: } EGLNativeWindowType;
On 2013/09/12 10:04:20, aknight wrote:
> I would prefer that the standard C++ pointer be used here for ICoreWindow, and
> that it be the only native handle (no struct). That keeps things compatible
with
> C/C++ (non-CX) applications, and doesn't take anything away from CX apps (they
> can still provide and use the pointer within ANGLE via standard C++). It also
> ensures you can build the lib without /ZW.
> 
> Having a struct as the window type is questionable because you can't easily
cast
> the pointer, and you don't always (perhaps often than not) use a
> swapchainbackground panel anyway. I'm not sure that the
swapchainbackgroundpanel
> pointer is really needed, and it causes additional issues when you consider
that
> Windows 8.1 adds new XAML swap chain panel types as well.
> 
> This would also reduce the need to include those headers (if you forward
declare
> the pointer types).

I wholeheartedly agree with these comments. It's not appropriate to define
EGLNativeWindowType as a complex type on WinRT, when in general it's expected to
be a lightweight handle, as on other platforms. IUnknown* would be my
preference.

https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newc...
include/EGL/eglplatform.h:94: #endif // ANGLE_PLATFORM_WINRT
On 2013/09/12 10:04:20, aknight wrote:
> The WINRT defs should go in a separate block instead of inside the Windows
> block, IMO.

Agreed. I would also strongly prefer not to have ANGLE-named guards in
eglplatform.h-- as our changes to this file should be contributed upstream for
consideration for inclusion in the standard version of the file. The guards
should use _WINRT or whatever the identifier on that platform is.

[aknight, 2013-09-12 14:42:59]

> What would happen if the application attempted to call
> glCompileShader/glLinkProgram on Windows Phone or WinRT 8.0? In order to
> maintain compliance, we need not only to avoid adding functionality to the API
> that is not there, but we also must support all API entry points as described
in
> the spec. We can't dictate to developers that they must load shaders only via
> glProgramBinaryOES, because the spec still contains the on-demand compile
path.
> (On the other hand, we'd technically be free to drop glProgramBinaryOES, as
it's
> added by an extension, so its support is optional.)

I assume that shader compilation should fail with e.g. GL_INVALID_OPERATION, and
GL_COMPILE_STATUS will be GL_FALSE. A meaningful error message (e.g. "the shader
compiler was not found/loaded") would be in the log.

A patch like this should be simple enough to keep on top of ANGLE without
requiring integration into mainline.

[stammen, 2013-09-13 13:28:05]

@aknight - Thank you for your comments. We are working on implementing
IUnknown* for the WinRT and Windows Phone EGLNativeWindowType and disabling
/Zw in the project files as it is no longer needed. We will resubmit the
header file for review in a few days.



On Thu, Sep 12, 2013 at 3:24 AM, <andrew.knight@digia.com> wrote:

>
> https://codereview.appspot.**com/12917046/diff/1/include/**
>
EGL/eglplatform.h<https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h>
> File include/EGL/eglplatform.h (right):
>
> https://codereview.appspot.**com/12917046/diff/1/include/**
>
EGL/eglplatform.h#newcode86<https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newcode86>
> include/EGL/eglplatform.h:86:
> Looking at this again, EGLNativeWindowType should really be an IUnknown
> pointer. That way, ANGLE can QueryInterface() for either ICoreWindow or
> ISwapChainBackgroundPanelNativ**e, and make the right swap chain choice
> internally. This also means that future D3D swap chain devices can be
> handled without modifying this header.
>
>
https://codereview.appspot.**com/12917046/<https://codereview.appspot.com/129...
>

[nikizh_gmail.com, 2013-09-27 10:01:07]

Hi,

Do you plan any support for SwapChainPanel ( 
http://msdn.microsoft.com/library/windows/apps/dn252834 / Win 8.1 only) ?

On Friday, September 13, 2013 4:27:34 PM UTC+3, Dale Stammen wrote:
>
> @aknight - Thank you for your comments. We are working on implementing 
> IUnknown* for the WinRT and Windows Phone EGLNativeWindowType and 
> disabling /Zw in the project files as it is no longer needed. We will 
> resubmit the header file for review in a few days.
>
>
>
> On Thu, Sep 12, 2013 at 3:24 AM, <andrew...@digia.com <javascript:>>wrote:
>
>>
>> https://codereview.appspot.**com/12917046/diff/1/include/**
>>
EGL/eglplatform.h<https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h>
>> File include/EGL/eglplatform.h (right):
>>
>> https://codereview.appspot.**com/12917046/diff/1/include/**
>>
EGL/eglplatform.h#newcode86<https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h#newcode86>
>> include/EGL/eglplatform.h:86:
>> Looking at this again, EGLNativeWindowType should really be an IUnknown
>> pointer. That way, ANGLE can QueryInterface() for either ICoreWindow or
>> ISwapChainBackgroundPanelNativ**e, and make the right swap chain choice
>> internally. This also means that future D3D swap chain devices can be
>> handled without modifying this header.
>>
>>
https://codereview.appspot.**com/12917046/<https://codereview.appspot.com/129...
>>
>
>

[aknight, 2013-09-27 11:01:28]

@nikizh_gmail.com	
> Do you plan any support for SwapChainPanel ( 
> http://msdn.microsoft.com/library/windows/apps/dn252834 / Win 8.1 only) ?

Just a quick look the API suggests that this is entirely possible. Some
pseudocode:

IUnknown *window
if window->QueryInterface(ICoreWindow)
 : createSwapChainForCoreWindow(window)
else window->QueryInterface(ISwapChainBackgroundPanelNative) ||
window->QueryInterface(ISwapChainPanelNative)
 : createSwapChainForComposition(window)
 : window->SetSwapChain(swapChain)

Of course, this requires compile-time guards for the Windows 8.1 SDK

[shaderschubert_gmail.com, 2013-11-12 12:42:33]

I implemented support for SwapChainPanel just today and it is just a tiny 
change to get it working per se. To get the correct resolution and and 
running outside of the UI Thread which is the preferred way of rendering to 
a SwapChainPanel was not possible without breaking the GL API Interface so 
far. I currently do not see a way to get it done without a hacky solution.

Am Freitag, 27. September 2013 12:01:05 UTC+2 schrieb nik...@gmail.com:
>
> Hi,
>
> Do you plan any support for SwapChainPanel ( 
> http://msdn.microsoft.com/library/windows/apps/dn252834 / Win 8.1 only) ?
>
> On Friday, September 13, 2013 4:27:34 PM UTC+3, Dale Stammen wrote:
>>
>> @aknight - Thank you for your comments. We are working on implementing 
>> IUnknown* for the WinRT and Windows Phone EGLNativeWindowType and 
>> disabling /Zw in the project files as it is no longer needed. We will 
>> resubmit the header file for review in a few days.
>>
>>
>>
>> On Thu, Sep 12, 2013 at 3:24 AM, <andrew...@digia.com> wrote:
>>
>>>
>>> https://codereview.appspot.com/12917046/diff/1/include/EGL/eglplatform.h
>>> File include/EGL/eglplatform.h (right):
>>>
>>> https://codereview.appspot.com/12917046/diff/1/include/
>>> EGL/eglplatform.h#newcode86
>>> include/EGL/eglplatform.h:86:
>>> Looking at this again, EGLNativeWindowType should really be an IUnknown
>>> pointer. That way, ANGLE can QueryInterface() for either ICoreWindow or
>>> ISwapChainBackgroundPanelNative, and make the right swap chain choice
>>> internally. This also means that future D3D swap chain devices can be
>>> handled without modifying this header.
>>>
>>> https://codereview.appspot.com/12917046/
>>>
>>
>>