Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-08 Thread Rob Shearman
2010/1/8 Wolfram Sang :
> Geoffrey Hausheer wrote:
>
>> Would someone mind reviewing this for style/content?  I still have no
>
> Can't say much about the content, but stylewise the pointer handling
> could be improved. For example,
>
> +    if ( (iface==0) || (ppvObject==0) )
>
> should at least use NULL instead of 0 as both are pointers, but !ptr is
> even better here. Accordingly

iface shouldn't generally be checked for NULL unless a program depends
on it and a testcase shows this behaviour in native.

-- 
Rob Shearman




Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-08 Thread Wolfram Sang
Geoffrey Hausheer wrote:

> Would someone mind reviewing this for style/content?  I still have no

Can't say much about the content, but stylewise the pointer handling
could be improved. For example,

+if ( (iface==0) || (ppvObject==0) )

should at least use NULL instead of 0 as both are pointers, but !ptr is
even better here. Accordingly

+*ppvObject = 0;

should assign NULL.

Also pointer declaration is not consistent, note the asterisk:

+  OLEPropertyPageSite *this = (OLEPropertyPageSite *)iface;

[...]

+  IPropertyPageSite*  iface,

(and there are two spaces here)

I don't know which way is preferred, but for readability it should be
the same throughout the file.

+if (IsEqualGUID(&IID_IUnknown, riid))
+*ppvObject = iface;
+else if (IsEqualGUID(&IID_IPropertyPageSite, riid))
+*ppvObject = iface;

if (a || b) ?

+static ULONG WINAPI PPS_Release(IPropertyPageSite* iface)
+{
+  OLEPropertyPageSite *this = (OLEPropertyPageSite *)iface;
+  ULONG ret;
+  TRACE("(%p)->(ref=%d)\n", this, this->ref);
+
+  /* Decrease the reference count for current interface */
+  ret = InterlockedDecrement(&this->ref);
+
+  return ret;
+}

ret is not needed here.

+/* This was expected to work but didn't:
+LONG baseunits = GetDialogBaseUnits();
+basex = LOWORD(baseunits);
+basey = HIWORD(baseunits);
+*/

Don't know the common wine-sense here, but without syntax-highlighting,
I think it is easy to miss that this code is disabled. I'd suggest

 /*
  *
  */

commenting or something similar which is immediately visible.

Those are my 2 cents, thanks for the work!

Regards,

Wolfram





Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-07 Thread Geoffrey Hausheer
Ok, I have reimplemented OleCreatePropertyFrame() from scratch.  There
is no code from Hidenori's patch in mine, nor did I use it as a
reference (though of course I can't claim that I didn't learn anything
from his patch during my previous cleanup)

Would someone mind reviewing this for style/content?  I still have no
idea how to write a test for a modal dialog, so if a test is required
for submission, I'd really appreciate someone giving me pointers on
how to go about it.

There are some features I didn't implement because I didn't really
understand MDSN sufficiently (specifcially the IPropertyPageSite
stuff), but it is mostly complete.

.Geoff
From 07ca9bda0bd66e53cfbdc441b59fa4bc2e988171 Mon Sep 17 00:00:00 2001
From: Geoffrey Hausheer 
Date: Thu, 7 Jan 2010 22:45:28 -0800
Subject: Implement OleCreatePropertyFrame from scratch

---
 dlls/oleaut32/Makefile.in|1 +
 dlls/oleaut32/olepropframe.c |  343 ++
 dlls/oleaut32/stubs.c|   22 ---
 3 files changed, 344 insertions(+), 22 deletions(-)
 create mode 100644 dlls/oleaut32/olepropframe.c

diff --git a/dlls/oleaut32/Makefile.in b/dlls/oleaut32/Makefile.in
index a8ab7b4..d304257 100644
--- a/dlls/oleaut32/Makefile.in
+++ b/dlls/oleaut32/Makefile.in
@@ -17,6 +17,7 @@ C_SRCS = \
oleaut.c \
olefont.c \
olepicture.c \
+   olepropframe.c \
recinfo.c \
regsvr.c \
safearray.c \
diff --git a/dlls/oleaut32/olepropframe.c b/dlls/oleaut32/olepropframe.c
new file mode 100644
index 000..cd1841c
--- /dev/null
+++ b/dlls/oleaut32/olepropframe.c
@@ -0,0 +1,343 @@
+/*
+ * OleCreatePropertyFrame
+ *
+ * Copyright 2010  Geoffrey Hausheer
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ *
+ */
+
+#include "config.h"
+#include "wine/port.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define COBJMACROS
+#define NONAMELESSUNION
+#define NONAMELESSSTRUCT
+
+#include "winerror.h"
+#include "windef.h"
+#include "winbase.h"
+#include "winnls.h"
+#include "winreg.h"
+#include "winuser.h"
+#include "lzexpand.h"
+
+#include "wine/unicode.h"
+#include "objbase.h"
+#include "typelib.h"
+#include "wine/debug.h"
+#include "variant.h"
+#include "wine/list.h"
+#include "ocidl.h"
+#include "prsht.h"
+#include "olectl.h"
+
+WINE_DEFAULT_DEBUG_CHANNEL(olepropframe);
+
+typedef struct
+{
+  const IPropertyPageSiteVtbl* lpVtbl;
+  LCID lcid;
+  LONG ref;
+} OLEPropertyPageSite;
+
+typedef struct {
+struct {
+   DLGTEMPLATE dlg;
+   WORD menu;
+   WORD class;
+   WORD caption;
+   WORD font;
+} dialog;
+OLEPropertyPageSite pps;
+IPropertyPage  *propPage;
+} OLEPropertyFrame;
+
+static INT_PTR CALLBACK prop_sheet_proc(HWND hwnd, UINT msg, WPARAM wparam,
+  LPARAM lparam)
+{
+
+switch(msg)
+{
+case WM_INITDIALOG:
+  {
+RECT rect;
+HRESULT result;
+PROPSHEETPAGEW *psp = (PROPSHEETPAGEW *)lparam; 
+OLEPropertyFrame *opf = (OLEPropertyFrame *)psp->lParam;
+
+TRACE("(%p, %s, %ld, %p)\n", hwnd, "WM_INITDIALOG",
+  wparam, opf->propPage);
+ZeroMemory( &rect, sizeof(rect) );
+GetClientRect( hwnd, &rect );
+result = IPropertyPage_Activate(opf->propPage, hwnd, &rect, TRUE);
+if(result == S_OK) {
+result = IPropertyPage_Show(opf->propPage, SW_SHOW );
+if(result == S_OK) {
+SetWindowLongPtrW( hwnd, DWLP_USER, (LONG)opf);
+}
+}
+return FALSE;
+  }
+case WM_DESTROY:
+  {
+OLEPropertyFrame *opf = (OLEPropertyFrame *)GetWindowLongPtrW( hwnd, 
DWLP_USER );
+if (opf) {
+IPropertyPage_Show(opf->propPage, SW_HIDE);
+IPropertyPage_Deactivate(opf->propPage);
+SetWindowLongPtrW( hwnd, DWLP_USER, (LONG)NULL);
+}
+  }
+case WM_NOTIFY:
+  {
+OLEPropertyFrame *opf = (OLEPropertyFrame *)GetWindowLongPtrW( hwnd, 
DWLP_USER );
+NMHDR *nmhdr = (NMHDR *)lparam;
+if(! opf)
+  break;
+
+switch(nmhdr->code)
+{
+case PSN_APPLY:
+FIXME("(%p, %s, %s)\n", hwnd, "WM_NOTIFY", "PSN_APPLY");
+IPropertyPage_Apply(opf->pro

Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-04 Thread Roderick Colenbrander
On Mon, Jan 4, 2010 at 7:18 PM, Geoffrey Hausheer
 wrote:
> On Mon, Jan 4, 2010 at 9:06 AM, Wolfram Sang  wrote:
>>> task.  There doesn't appear to be anything in this code that would be
>>> patent encumbered, but the code itself certainly falls under
>>> Hidenori's copyright which he has effectively revoked permission to
>>> use.
>>
>> Strictly speaking, this is not possible. Although IANAL, this code is LGPL,
>> so he can't revoke permission for this version of the code. He can ask to
>> not use it, and this wish can be granted. You still should be allowed to
>> pick it up. Next question is then, if a modified version will be picked up
>> (still respecting the author's wish)? Or how much modifications are needed?
>> Geez, such things are never trivial :(
>>
>
> Well, I put the LGPL header on the code.  I thought I had that right
> since the code was posted to wine-patches  Anyhow, the legal side
> isn't really relevant.  The question is what Alexandre will accept as
> far as it goes.  The current code is certainly copyright by Hienori,
> and since I couldn't contact him, I must assume he doesn't want any of
> it in Wine.  Thus my assumption is that I need to rewrite the entire
> thing, if we want to obey his wishes (which I do).  However, my hope
> was that I could use the knowledge I gained from his implementation to
> do it myself.  As I'm already biased by that knowledge, if that won't
> be allowed, then I can't do the work at all.
>

As far as I remember Hidenori asked for removal of his Quartz work
because he was affright of patents, Microsoft. He didn't ask for
removal of his other code (perhaps he did in a private mail) but I
think Alexandre also removed his other code because it is not worth to
take any risks.

Recently we had a case which is a little bit similar. In that case
someone wrote some dirty tests to figure out what Windows was doing
and used that knowledge to write Wine code. The code in question was
rejected but the guy also wasn't allowed to use his knowledge to
submit a wine test.

I'm quite certain that you can't resubmit Hidenori's code because it
was removed for a good reason. I'm not sure about tests though but I
guess we don't want to take any risks with that either :( Wait for
Alexandre his answer though.

Roderick




Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-04 Thread Geoffrey Hausheer
On Mon, Jan 4, 2010 at 9:06 AM, Wolfram Sang  wrote:
>> task.  There doesn't appear to be anything in this code that would be
>> patent encumbered, but the code itself certainly falls under
>> Hidenori's copyright which he has effectively revoked permission to
>> use.
>
> Strictly speaking, this is not possible. Although IANAL, this code is LGPL,
> so he can't revoke permission for this version of the code. He can ask to
> not use it, and this wish can be granted. You still should be allowed to
> pick it up. Next question is then, if a modified version will be picked up
> (still respecting the author's wish)? Or how much modifications are needed?
> Geez, such things are never trivial :(
>

Well, I put the LGPL header on the code.  I thought I had that right
since the code was posted to wine-patches  Anyhow, the legal side
isn't really relevant.  The question is what Alexandre will accept as
far as it goes.  The current code is certainly copyright by Hienori,
and since I couldn't contact him, I must assume he doesn't want any of
it in Wine.  Thus my assumption is that I need to rewrite the entire
thing, if we want to obey his wishes (which I do).  However, my hope
was that I could use the knowledge I gained from his implementation to
do it myself.  As I'm already biased by that knowledge, if that won't
be allowed, then I can't do the work at all.





Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-04 Thread Wolfram Sang



task.  There doesn't appear to be anything in this code that would be
patent encumbered, but the code itself certainly falls under
Hidenori's copyright which he has effectively revoked permission to
use.


Strictly speaking, this is not possible. Although IANAL, this code is 
LGPL, so he can't revoke permission for this version of the code. He can 
ask to not use it, and this wish can be granted. You still should be 
allowed to pick it up. Next question is then, if a modified version will 
be picked up (still respecting the author's wish)? Or how much 
modifications are needed? Geez, such things are never trivial :(


Regards,

   Wolfram





Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-04 Thread Geoffrey Hausheer
On Mon, Jan 4, 2010 at 4:39 AM, John Klehm wrote:
> On Mon, Jan 4, 2010 at 4:46 AM, Huw Davies  wrote:
>> On Sun, Jan 03, 2010 at 03:00:12PM -0800, Geoffrey Hausheer wrote:
>>> I found a patch from 2001 written by TAKESHIMA Hidenori that
>>> was posted to wine-patches
>>> (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html)
>>> but never committed.
>>
>> Actually it was commited (2843934af5515c7f2b8370324aa98d3964a40324)
>> but was then removed (140e7222e0d7ce76068cddc64c68105c2e569257) at
>> his request.
>>
>
> In case anyone was just slightly less curious than me about it:
> http://www.winehq.org/wwn/122#Quartz.dll%20Removal
>
Thanks for turning that up.  I did look through the archives to see
what hapened, but aparently not hard enough.
So now the only solution appears to be to rewrite the code from
scratch.  The question is, can I do that now that I've looked over his
code?  It does turn this from a simple fix to a much more daunting
task.  There doesn't appear to be anything in this code that would be
patent encumbered, but the code itself certainly falls under
Hidenori's copyright which he has effectively revoked permission to
use.

.Geoff





Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-04 Thread John Klehm
On Mon, Jan 4, 2010 at 4:46 AM, Huw Davies  wrote:
> On Sun, Jan 03, 2010 at 03:00:12PM -0800, Geoffrey Hausheer wrote:
>> I found a patch from 2001 written by TAKESHIMA Hidenori that
>> was posted to wine-patches
>> (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html)
>> but never committed.
>
> Actually it was commited (2843934af5515c7f2b8370324aa98d3964a40324)
> but was then removed (140e7222e0d7ce76068cddc64c68105c2e569257) at
> his request.
>

In case anyone was just slightly less curious than me about it:
http://www.winehq.org/wwn/122#Quartz.dll%20Removal

--John Klehm




Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-04 Thread Huw Davies
On Sun, Jan 03, 2010 at 03:00:12PM -0800, Geoffrey Hausheer wrote:
> I found a patch from 2001 written by TAKESHIMA Hidenori that
> was posted to wine-patches
> (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html)
> but never committed.

Actually it was commited (2843934af5515c7f2b8370324aa98d3964a40324)
but was then removed (140e7222e0d7ce76068cddc64c68105c2e569257) at
his request.

Huw.




Re: Patch feedback requested for OleCreatePropertyFrame()

2010-01-03 Thread Maarten Lankhorst
Hi Geoffrey,

2010/1/4 Geoffrey Hausheer :
> Some software I'm using is configured via an OleCreatePropertyFrame
> call which isn't implemented in Wine (ticket #16564).  I can use
> winetricks + dcom98 to get it working, but would prefer not to need
> that.  I found a patch from 2001 written by TAKESHIMA Hidenori that
> was posted to wine-patches
> (http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html)
> but never committed.  I cleaned up the patch to compile against
> current Git and am including it here for inspection.  I can't find any
> documentation on why it wasn't included at that time, and haven't done
> any Wine development for a long time, so I'm not sure what it will
> take to get it included.
>
> I have tested it, and it works pretty much fine for me (not quite as
> pretty as the dcom menu, but it is functional).  I'd be happy to write
> a test for it if I had any idea how to do that, but I'm not sure where
> to begin for a function like this.
>
> Anyhow, I'm open to feedback on what should be done before I submit
> the patch for inclusion
>
> FYI: I was unable to find a currently valid email address for Takeshima
Those defines bother me:

+#define CPropertyPageContainerImpl_AddRef(pContainer)
(++((pContainer)->ref))
+#define CPropertyPageContainerImpl_Release(pContainer)
(--((pContainer)->ref))
Interlocked* functions?

The other define looks ugly, testcases are missing, and A functions
are used, rather than the W functions, which is probably what blocked
this.

Some of the FIXME messages should really be warns too.

Cheers,
Maarten.




Patch feedback requested for OleCreatePropertyFrame()

2010-01-03 Thread Geoffrey Hausheer
Some software I'm using is configured via an OleCreatePropertyFrame
call which isn't implemented in Wine (ticket #16564).  I can use
winetricks + dcom98 to get it working, but would prefer not to need
that.  I found a patch from 2001 written by TAKESHIMA Hidenori that
was posted to wine-patches
(http://www.winehq.org/pipermail/wine-patches/2001-October/001229.html)
but never committed.  I cleaned up the patch to compile against
current Git and am including it here for inspection.  I can't find any
documentation on why it wasn't included at that time, and haven't done
any Wine development for a long time, so I'm not sure what it will
take to get it included.

I have tested it, and it works pretty much fine for me (not quite as
pretty as the dcom menu, but it is functional).  I'd be happy to write
a test for it if I had any idea how to do that, but I'm not sure where
to begin for a function like this.

Anyhow, I'm open to feedback on what should be done before I submit
the patch for inclusion

FYI: I was unable to find a currently valid email address for Takeshima

.Geoff
From 6fece5fb94617f00345a7116f6a40b783d1c1466 Mon Sep 17 00:00:00 2001
From: Geoffrey Hausheer 
Date: Sun, 3 Jan 2010 14:22:25 -0800
Subject: Add support for OleCreatePropertyFrame

This patch was originally posted in 2001 by TAKESHIMA Hidenori
---
 dlls/oleaut32/Makefile.in |1 +
 dlls/oleaut32/propertyframe.c |  602 +
 dlls/oleaut32/stubs.c |   22 --
 3 files changed, 603 insertions(+), 22 deletions(-)
 create mode 100644 dlls/oleaut32/propertyframe.c

diff --git a/dlls/oleaut32/Makefile.in b/dlls/oleaut32/Makefile.in
index a8ab7b4..493d4eb 100644
--- a/dlls/oleaut32/Makefile.in
+++ b/dlls/oleaut32/Makefile.in
@@ -17,6 +17,7 @@ C_SRCS = \
oleaut.c \
olefont.c \
olepicture.c \
+   propertyframe.c \
recinfo.c \
regsvr.c \
safearray.c \
diff --git a/dlls/oleaut32/propertyframe.c b/dlls/oleaut32/propertyframe.c
new file mode 100644
index 000..96b4e67
--- /dev/null
+++ b/dlls/oleaut32/propertyframe.c
@@ -0,0 +1,602 @@
+/*
+ * Copyright 2001 TAKESHIMA Hidenori 
+ * Copyright 2010 Geoffrey Hausheer 
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ *
+ * 
--
+ * Known problems (2001, TAKESHIMA Hidenori)
+ *   Should use PropertySheetW rather than PropertySheetA
+ */
+
+#include "config.h"
+#include "wine/port.h"
+
+#include 
+
+#define COBJMACROS
+#define NONAMELESSUNION
+#define NONAMELESSSTRUCT
+
+#include "winerror.h"
+#include "windef.h"
+#include "winbase.h"
+#include "wingdi.h"
+#include "winuser.h"
+#include "ole2.h"
+#include "olectl.h"
+#include "oleauto.h"
+#include "commctrl.h"
+#include "wine/debug.h"
+
+WINE_DEFAULT_DEBUG_CHANNEL(ole);
+
+typedef struct CPropertyPageContainerImpl CPropertyPageContainerImpl;
+
+static const struct
+{
+DLGTEMPLATEtempl;
+WORDwMenuName;
+WORDwClassName;
+WCHARwDummyCaption;
+BYTEpadding[4];
+} propsite_dlg =
+{
+{
+WS_VISIBLE | WS_CHILD, /* style */
+0, /* dwExtendedStyle */
+0, /* cdit */
+0, /* x */
+0, /* y */
+0, /* cx */
+0, /* cy */
+},
+0, 0, 0,
+};
+
+typedef struct CPropertyPageSiteImpl
+{
+const IPropertyPageSiteVtbl *lpVtbl;/* Needs to be the first item 
in the struct */
+/* IUnknown fields */
+LONGref;
+
+/* IPropertyPageSite fields */
+CPropertyPageContainerImpl*pContainer;
+IPropertyPage*pPage;
+HWNDhwnd;
+BYTEtempl[sizeof(propsite_dlg)];
+PROPPAGEINFOinfo;
+BOOLbActivate;
+} CPropertyPageSiteImpl;
+
+struct CPropertyPageContainerImpl
+{
+ULONGref; /* for IUnknown(not used now) */
+LCIDlcid;
+DWORDm_cSites;
+CPropertyPageSiteImpl**m_ppSites;
+PROPSHEETPAGEA*m_pPsp;
+HRESULT m_hr;
+};
+
+/* for future use. */
+#define CPropertyPageContainerImpl_AddRef(pContainer)
(++((pContainer)->ref))
+#define CPropertyPageContainerImpl_Release(pContainer)
(--((pContainer)->ref))
+
+
+/***/
+
+#define PropSiteDlg_Return

Re: Patch feedback (GetProductInfo)

2009-07-03 Thread Alexander Nicolaysen Sørnes
Fredag 03 juli 2009 11:10:01 skrev Nikolay Sivov:
> Alexander Nicolaysen Sørnes wrote:
> > Hello,
> >
> > Could someone give me a hint as to what is wrong with the following
> > patches?
> >
> > http://www.winehq.org/pipermail/wine-patches/2009-June/074908.html
> > http://www.winehq.org/pipermail/wine-patches/2009-June/074909.html
> >
> > They only adds some defines plus a stub, so hopefully there isn't that
> > much I can have done wrong :)
> >
> >
> >
> > Alexander N. Sørnes
>
> Hi, Alexander.
>
> First of all constant defines should go to winnt.h to match PSDK.
> Next I found RtlGetProductInfo() call in winnt.h. That's make me think
> that kernel32 call should be a thin wrapper
> over this ntdll call. I don't have Vista installed (I suppose it's Vista
> related call), so I can't check.
>
> After that it might be nice to have a combo in winecfg to choose edition
> - as I know upcoming Win7 also has
> this product categories. Default value could be Ultimate of course. And
> combo should only appear on >= Vista.
>
> Nikolay S.


Thanks!




Re: Patch feedback (GetProductInfo)

2009-07-03 Thread Nikolay Sivov

Alexander Nicolaysen Sørnes wrote:

Hello,

Could someone give me a hint as to what is wrong with the following patches?

http://www.winehq.org/pipermail/wine-patches/2009-June/074908.html
http://www.winehq.org/pipermail/wine-patches/2009-June/074909.html

They only adds some defines plus a stub, so hopefully there isn't that much I 
can have done wrong :)




Alexander N. Sørnes
  

Hi, Alexander.

First of all constant defines should go to winnt.h to match PSDK.
Next I found RtlGetProductInfo() call in winnt.h. That's make me think 
that kernel32 call should be a thin wrapper
over this ntdll call. I don't have Vista installed (I suppose it's Vista 
related call), so I can't check.


After that it might be nice to have a combo in winecfg to choose edition 
- as I know upcoming Win7 also has
this product categories. Default value could be Ultimate of course. And 
combo should only appear on >= Vista.


Nikolay S.





Patch feedback (GetProductInfo)

2009-07-02 Thread Alexander Nicolaysen Sørnes
Hello,

Could someone give me a hint as to what is wrong with the following patches?

http://www.winehq.org/pipermail/wine-patches/2009-June/074908.html
http://www.winehq.org/pipermail/wine-patches/2009-June/074909.html

They only adds some defines plus a stub, so hopefully there isn't that much I 
can have done wrong :)



Alexander N. Sørnes




Re: MSI Patch Feedback

2009-01-09 Thread James Hawkins
On Fri, Jan 9, 2009 at 9:52 AM, ricardo filipe
 wrote:
>
>
>>>"How should I attempt to test the changes I have made"
>> I don't see testcases for any of these changes. Adding conformance
>> tests would be the best first course of action:
>>
>> http://wiki.winehq.org/ConformanceTests
>> http://wiki.winehq.org/WritingConformanceTests
>>
>> --
>> -Austin
>>
>>
>
> he is asking how he should test... i think he knows he has to do them :)
> i had the same problem a while back and got no response too so it's on 
> standby...
>

There is no way to test the setting of environment variables by msi.
You can't query the changed environment variable until the process
exits.  Using a child process doesn't help either.

-- 
James Hawkins




RE: MSI Patch Feedback

2009-01-09 Thread ricardo filipe


>>"How should I attempt to test the changes I have made"
> I don't see testcases for any of these changes. Adding conformance
> tests would be the best first course of action:
>
> http://wiki.winehq.org/ConformanceTests
> http://wiki.winehq.org/WritingConformanceTests
>
> --
> -Austin
>
>

he is asking how he should test... i think he knows he has to do them :) 
i had the same problem a while back and got no response too so it's on 
standby...
_
Show them the way! Add maps and directions to your party invites. 
http://www.microsoft.com/windows/windowslive/events.aspx



Re: MSI Patch Feedback

2009-01-09 Thread Austin English
On Fri, Jan 9, 2009 at 3:18 AM, Andrew Nguyen  wrote:
> Hello,
>
> I've written a few patches against dlls/msi/action.c in an effort to
> rectify a difference in behavior with an MSI-based installer between
> Windows and Wine with regard to environment variable processing.
> Behavior not explicitly enumerated by MSDN is noted when a combination
> of the environment table "+" name prefix and "[~]" value expression
> are combined. How should I attempt to test the changes I have made,
> and is there anything I can do to improve the quality of my changes?
> Feedback is appreciated!
>
> -Andrew
>
>
>
>

I don't see testcases for any of these changes. Adding conformance
tests would be the best first course of action:

http://wiki.winehq.org/ConformanceTests
http://wiki.winehq.org/WritingConformanceTests

-- 
-Austin




MSI Patch Feedback

2009-01-09 Thread Andrew Nguyen
Hello,

I've written a few patches against dlls/msi/action.c in an effort to
rectify a difference in behavior with an MSI-based installer between
Windows and Wine with regard to environment variable processing.
Behavior not explicitly enumerated by MSDN is noted when a combination
of the environment table "+" name prefix and "[~]" value expression
are combined. How should I attempt to test the changes I have made,
and is there anything I can do to improve the quality of my changes?
Feedback is appreciated!

-Andrew


msipatches.tar.bz2
Description: BZip2 compressed data



Re: Patch Feedback

2008-12-19 Thread Jacek Caban
Hi Alistair,

Alistair Leslie-Hughes wrote:
> Hi,
>
> Is there anything wrong with patches?
>
> [1/2] mshtml: Implement IHTMLScriptElement get/put event
>
> [2/2] mshtml: Implement IHTMLScriptElement get/put htmlFor
>
>   

First of all, are you trying to fix a real bug? We're starting to handle 
scripts ourselves, so your implementation is not complete. It would be 
fine to add such semi-stubs if it fixes a real bug. Otherwise I'd prefer 
to leave them as stubs so we don't hide bugs.

Also, we have NS_FAILED macro that you should use instead of 
!NS_SUCCEEDED and printing a message on failure would be nice.


Jacek




Patch Feedback

2008-12-18 Thread Alistair Leslie-Hughes
Hi,

Is there anything wrong with patches?

[1/2] mshtml: Implement IHTMLScriptElement get/put event

[2/2] mshtml: Implement IHTMLScriptElement get/put htmlFor



Best Regards

 Alistair Leslie-Hughes






Re: Patch feedback

2008-07-06 Thread Eric Pouech
Lei Zhang a écrit :
> On Sun, Jul 6, 2008 at 6:55 AM, Alexander Nicolaysen Sørnes
> <[EMAIL PROTECTED]> wrote:
>   
>> Hello,
>>
>> I would really appreciate any feedback as to how these patches can be 
>> improved
>> so that they will be accepted into Wine.
>>
>> regedit: Add support for importing Unicode files
>> http://www.winehq.org/pipermail/wine-patches/2008-June/056856.html
>>
>> I know this patch is a little big, but I'm not sure how it could be split up
>> without possible data loss on import.  Is that acceptable?  I could for
>> instance make a first patch that merely reads the file as Uncode then
>> converts each line to multibyte characters.
>> 
>
> Sure. How about:
>
> 1 patch for the first two utility functions.
> 1 patch for each W version of an existing function.
> 1 or 2 patches to glue it all together.
>
>   
duplicating a lot of functions for A/W compatibility just shows that the 
internals of regedit should be moved to unicode first
then your patch will be relatively straightforward
A+

-- 
Eric Pouech
"The problem with designing something completely foolproof is to underestimate 
the ingenuity of a complete idiot." (Douglas Adams)






Re: Patch feedback

2008-07-06 Thread Lei Zhang
On Sun, Jul 6, 2008 at 6:55 AM, Alexander Nicolaysen Sørnes
<[EMAIL PROTECTED]> wrote:
> Hello,
>
> I would really appreciate any feedback as to how these patches can be improved
> so that they will be accepted into Wine.
>
> regedit: Add support for importing Unicode files
> http://www.winehq.org/pipermail/wine-patches/2008-June/056856.html
>
> I know this patch is a little big, but I'm not sure how it could be split up
> without possible data loss on import.  Is that acceptable?  I could for
> instance make a first patch that merely reads the file as Uncode then
> converts each line to multibyte characters.

Sure. How about:

1 patch for the first two utility functions.
1 patch for each W version of an existing function.
1 or 2 patches to glue it all together.




Patch feedback

2008-07-06 Thread Alexander Nicolaysen Sørnes
Hello,

I would really appreciate any feedback as to how these patches can be improved 
so that they will be accepted into Wine.

regedit: Add support for importing Unicode files
http://www.winehq.org/pipermail/wine-patches/2008-June/056856.html

I know this patch is a little big, but I'm not sure how it could be split up 
without possible data loss on import.  Is that acceptable?  I could for 
instance make a first patch that merely reads the file as Uncode then 
converts each line to multibyte characters.


user32: Always use old-style colours for Windows 3.x
http://www.winehq.org/pipermail/wine-patches/2008-June/056898.html



Alexander N. Sørnes




Re: patch feedback?

2006-08-11 Thread Alexandre Julliard
Damjan Jovanovic <[EMAIL PROTECTED]> writes:

> +classKey = SetupDiOpenClassRegKeyExW(&guid, KEY_ALL_ACCESS,
> + DIOCR_INSTALLER, NULL, NULL);
> +/* wine does incorrectly return FALSE, so ... */
> +ok(classKey != FALSE && classKey != INVALID_HANDLE_VALUE,
> +"failed opening class key, error %ld\n", GetLastError());

Tests are not supposed to cope with Wine bugs, that defeats the whole
purpose of the test. If returning FALSE is wrong then the test should
fail in that case.

-- 
Alexandre Julliard
[EMAIL PROTECTED]




patch feedback?

2006-08-11 Thread Damjan Jovanovic
Hi

I've tried submitting a patch to wine-patches 3 times
now in the past 3 weeks, and not only has it not been
applied, but I never even got any feedback.

Please tell me what I am doing wrong.

The latest patch is below.

Thank you
Damjan

--- Damjan Jovanovic <[EMAIL PROTECTED]> wrote:

> Date: Thu, 3 Aug 2006 23:49:11 -0700 (PDT)
> From: Damjan Jovanovic <[EMAIL PROTECTED]>
> To: [EMAIL PROTECTED]
> Subject: setupapi: test+patch for
> SetupDiOpenClassRegKeyExW (try 3)
> 
> wine's SetupDiOpenClassRegKeyExW returns FALSE
> instead
> of INVALID_HANDLE_VALUE in several places, and
> doesn't
> brace the GUID string like it should. The test
> reveals
> these problems, and the patch fixes them.
> 
> ChangeLog:
> * Fixed returns values, braced GUID string, and
> added
> a test, for SetupDiOpenClassRegKeyExW.
> 
> Damjan Jovanovic ([EMAIL PROTECTED])
> 
> 
> a/dlls/setupapi/tests/Makefile.in 2006-07-10
> 18:01:08.0 +0200
> +++ b/dlls/setupapi/tests/Makefile.in 2006-08-04
> 16:15:05.0 +0200
> @@ -3,7 +3,7 @@
>  SRCDIR= @srcdir@
>  VPATH = @srcdir@
>  TESTDLL   = setupapi.dll
> -IMPORTS   = setupapi kernel32
> +IMPORTS   = setupapi kernel32 advapi32 rpcrt4
>  
>  CTESTS = \
>   devinst.c \
> --- a/dlls/setupapi/tests/devinst.c   2006-08-03
> 10:07:11.0 +0200
> +++ b/dlls/setupapi/tests/devinst.c   2006-08-04
> 17:57:57.0 +0200
> @@ -27,6 +27,7 @@
>  #include "winuser.h"
>  #include "winreg.h"
>  #include "setupapi.h"
> +#include "rpc.h"
>  
>  #include "wine/test.h"
>  
> @@ -63,7 +64,85 @@
>  ok(ret, "SetupDiDestroyDeviceInfoList failed :
> %ld\n", error);
>  }
>  
> +static void test_SetupDiOpenClassRegKeyExW()
> +{
> +GUID guid;
> +RPC_STATUS rpcStatus;
> +HKEY hkey;
> +WCHAR *unbracedGuid;
> +WCHAR *bracedGuid;
> +
> +rpcStatus = UuidCreate(&guid);
> +if (rpcStatus != RPC_S_OK)
> +{
> +ok(FALSE, "failed to generate test guid,
> error %ld", rpcStatus);
> +return;
> +}
> +
> +/* Check return value for non-existant key */
> +hkey = SetupDiOpenClassRegKeyExW(&guid,
> KEY_ALL_ACCESS, 
> +DIOCR_INSTALLER, NULL, NULL);
> +ok(hkey == INVALID_HANDLE_VALUE,
> +"invalid return value %p from
> SetupDiOpenClassRegKeyExW "
> +"for non-existant key, expected %p\n",
> hkey, INVALID_HANDLE_VALUE);
> +
> +rpcStatus = UuidToStringW(&guid,
> &unbracedGuid);
> +if (rpcStatus != RPC_S_OK)
> +{
> +ok(FALSE, "failed to get string form of
> guid, error %ld", rpcStatus);
> +return;
> +}
> +
> +bracedGuid = HeapAlloc(GetProcessHeap(), 0,
> +(lstrlenW(unbracedGuid) + 3) *
> sizeof(WCHAR));
> +if (bracedGuid != NULL)
> +{
> +HKEY classesKey;
> +
> +bracedGuid[0] = (WCHAR) '{';
> +memcpy(&bracedGuid[1], unbracedGuid,
> +lstrlenW(unbracedGuid) *
> sizeof(WCHAR));
> +bracedGuid[1 + lstrlenW(unbracedGuid)] =
> (WCHAR) '}';
> +bracedGuid[1 + lstrlenW(unbracedGuid) + 1]
> = (WCHAR) 0;
> +
> +SetLastError(0xdeadbeef);
> +classesKey =
> SetupDiOpenClassRegKeyExW(NULL, KEY_ALL_ACCESS,
> +DIOCR_INSTALLER, NULL, NULL);
> +ok(classesKey != INVALID_HANDLE_VALUE,
> +"failed to open the device classes
> registry key: error %ld\n",
> +GetLastError());
> +if (classesKey != INVALID_HANDLE_VALUE)
> +{
> +DWORD ret;
> +HKEY classKey;
> +ret = RegCreateKeyW(classesKey,
> bracedGuid, &classKey);
> +if (ret == ERROR_SUCCESS)
> +{
> +RegCloseKey(classKey);
> +SetLastError(0xdeadbeef);
> +classKey =
> SetupDiOpenClassRegKeyExW(&guid, KEY_ALL_ACCESS,
> + DIOCR_INSTALLER, NULL, NULL);
> +/* wine does incorrectly return
> FALSE, so ... */
> +ok(classKey != FALSE && classKey !=
> INVALID_HANDLE_VALUE,
> +"failed opening class key,
> error %ld\n", GetLastError());
> +RegCloseKey(classKey);
> +RegDeleteKeyW(classesKey,
> bracedGuid);
> +}
> +else
> +ok(FALSE, "failed creating device
> key: error %ld\n", ret);
> +RegCloseKey(classesKey);
> +}
> +
> +HeapFree(GetProcessHeap(), 0, bracedGuid);
> +}
> +else
> +ok(FALSE, "failed to allocate memory for
> braced guid\n");
> +
> +RpcStringFreeW(&unbracedGuid);
> +}
> +
>  START_TEST(devinst)
>  {
>  test_SetupDiCreateDeviceInfoListEx();
> +test_SetupDiOpenClassRegKeyExW();
>  }
> --- a/dlls/setupapi/devinst.c 2006-08-03
> 10:07:11.0 +0200
> +++ b/dlls/setupapi/devinst.c 2006-08-04
> 18:10:06.0 +0200
> @@ -1393,6 +1393,7 @@
>  PVOID Reserved)
>  {
>  LPWSTR lpGuidString;
> +LPWSTR lpBracedGuidString;
>  HKEY hClassesKey;
>  HKEY hClassKey;
>  LPCWSTR lpKeyName;
> @@ -

Re: wined3d: GLSL Patch feedback requested

2006-06-13 Thread Raphael
On Tuesday 13 June 2006 18:39, Jason Green wrote:
> On 6/13/06, H. Verbeet <[EMAIL PROTECTED]> wrote:
> > On 13/06/06, Raphael <[EMAIL PROTECTED]> wrote:
> > > I only have one  question:
> > >  i see you have a cache to reuse prgid in set_glsl_shader_program but
> > > what happens if user/app change the shaders content (for example using
> > > SetFunction) ? your pre-compiled program will be correct ?
> > > (because i don't see any list cleanup in shaders function reset)
> >
> > Good point :-)
> > I wonder if respecifying the source and recompiling the program is
> > worth it, or if we should just create a new shader object.
>
> http://msdn.microsoft.com/library/default.asp?url=/library/en-us/directx9_c
>/IDirect3DPixelShader9.asp
>
> >From what I can tell (for DX9 at least), the app shouldn't be able to
>
> use SetFunction().  I thought it was an internal wined3d function.
> Unless it's a DX8 call that I'm unaware of.  I haven't seen any apps
> try to do that yet, but maybe I'm just not looking closely enough.
>
> :-)   The apps tend to CreatePixelShader(), then later [often during
>
> the render loop], they SetPixelShader().  I haven't seen any that
> modify an existing shader yet.

No seems correct to me (from DX8-9 APIs) but never say never :)
Anyway cleaning glprograms on Delete can be a good idea :)

And we must think, for futur, about the ressources manager (was last oliver 
works). As in some cases the CG will not be able to contains all precompiled 
shaders couples, and we must swap them on need  ;)

Best Regards and thx to you two,
Raphael


pgprt8voNzH1B.pgp
Description: PGP signature



Re: wined3d: GLSL Patch feedback requested

2006-06-13 Thread H. Verbeet

On 13/06/06, Jason Green <[EMAIL PROTECTED]> wrote:

From what I can tell (for DX9 at least), the app shouldn't be able to
use SetFunction().

You're right.




Re: wined3d: GLSL Patch feedback requested

2006-06-13 Thread Jason Green

On 6/13/06, H. Verbeet <[EMAIL PROTECTED]> wrote:

On 13/06/06, Raphael <[EMAIL PROTECTED]> wrote:
> I only have one  question:
>  i see you have a cache to reuse prgid in set_glsl_shader_program but what
> happens if user/app change the shaders content (for example using
> SetFunction) ? your pre-compiled program will be correct ?
> (because i don't see any list cleanup in shaders function reset)
Good point :-)
I wonder if respecifying the source and recompiling the program is
worth it, or if we should just create a new shader object.


http://msdn.microsoft.com/library/default.asp?url=/library/en-us/directx9_c/IDirect3DPixelShader9.asp


From what I can tell (for DX9 at least), the app shouldn't be able to

use SetFunction().  I thought it was an internal wined3d function.
Unless it's a DX8 call that I'm unaware of.  I haven't seen any apps
try to do that yet, but maybe I'm just not looking closely enough.
:-)   The apps tend to CreatePixelShader(), then later [often during
the render loop], they SetPixelShader().  I haven't seen any that
modify an existing shader yet.




Re: wined3d: GLSL Patch feedback requested

2006-06-13 Thread H. Verbeet

On 13/06/06, Raphael <[EMAIL PROTECTED]> wrote:

I only have one  question:
 i see you have a cache to reuse prgid in set_glsl_shader_program but what
happens if user/app change the shaders content (for example using
SetFunction) ? your pre-compiled program will be correct ?
(because i don't see any list cleanup in shaders function reset)

Good point :-)
I wonder if respecifying the source and recompiling the program is
worth it, or if we should just create a new shader object.


 - It is correct to have GLSL binding code out of "if
(useVertexShaderFunction)" block ?

It is, actually. If there's no shader in use, we need to tell GL to
use fixed function again. The glUseProgramObjectARB call takes care of
that. (programId is zero when no shaders are in use).




Re: wined3d: GLSL Patch feedback requested

2006-06-12 Thread Raphael
On Thursday 08 June 2006 17:35, Jason Green wrote:
> The current cumulative patch is located here:
>
> http://cmhousing.net/wine/glsl_cumul.diff
>

I hate you, reviewing all this patch made me crazy.

Anyway is an impressive work 

I only have one  question:
 i see you have a cache to reuse prgid in set_glsl_shader_program but what 
happens if user/app change the shaders content (for example using 
SetFunction) ? your pre-compiled program will be correct ?
(because i don't see any list cleanup in shaders function reset)

And many comments:

 void inline  drawPrimitiveDrawStrided(IWineD3DDevice *iface, BOOL 
useVertexShaderFunction, BOOL usePixelShaderFunction, int useHW, 
WineDirect3DVertexStridedData *dataLocations,
 UINT numberOfvertices, UINT numberOfIndicies, GLenum glPrimType, const void 
*idxData, short idxSize, int minIndex, long StartIdx) {
 IWineD3DDeviceImpl *This = (IWineD3DDeviceImpl *)iface;
@@ -1791,91 +1920,59 @@ #endif
 
 TRACE("Loaded arrays\n");
 
+/* Bind the correct GLSL shader program based on the currently set 
vertex & pixel shaders. */
+if (wined3d_settings.shader_mode == SHADER_GLSL) {
+GLhandleARB programId;
+
+set_glsl_shader_program(iface);
+programId = This->stateBlock->shaderPrgId;
+
+if (programId != 0) {
+/* Start using this program ID */
+TRACE_(d3d_shader)("Using GLSL program %u\n", programId);
+GL_EXTCALL(glUseProgramObjectARB(programId));
+checkGLcall("glUseProgramObjectARB");
+} 
+}
+
 if (useVertexShaderFunction) {
-

 - It is correct to have GLSL binding code out of "if 
(useVertexShaderFunction)" block ?

 - i think moving binding code (GLSL and ARB) on a new base shader method (ex 
IWineD3DBaseShaderImpl_Bind) will be cleaner (and permit to not have GLS/ARB 
code in drawprim)

 - same for shaders constant loading

Best Regards,
Raphael


pgpu82Xo7y16L.pgp
Description: PGP signature



Re: wined3d: GLSL Patch feedback requested

2006-06-08 Thread Ivan Gyurdiev

+ * If a program for the given combination does not exist, create one, and store
+ * that data in both shader objects so we can delete all of the programs later.
+ * If it creates a program, it will link the given objects, too.

Is this comment still relevant?



- Fix relative addressing (it was working correctly, then I lost some
stuff when my hard drive got corrupted, and now it's using R0 instead
of A0 as the address register for some reason)
For 1.X shaders, the relative address token = 0 (a.k.a R0), and A0.x 
must be used instead.
Another problem may be matrix instructions, which don't seem to set the 
reladdr token right now.






Re: wined3d: GLSL Patch feedback requested

2006-06-08 Thread Nick Burns

That looks very impressive (good progress).
I would have to look at the actual GLSL produced by this.

The actual translation points look good.


From: "Jason Green" <[EMAIL PROTECTED]>
Date: Thu, 8 Jun 2006 11:54:04 -0400

By the way, here's a comparison screenshot of Civ4 from before my hard
drive failed, and this is without having texturing on pixel shaders
entirely working.  Using GLSL (working completely for vertex shaders
2.0, at least the instructions that Civilization 4 used):

http://cmhousing.net/wine/civ4_glsl.png

compared to:

http://cmhousing.net/wine/civ4_ingame2.png
and
http://cmhousing.net/wine/civ4_3.png

with current git and using regular ARB shaders.

So, we at least know that this is going to help.  :-)


On 6/8/06, Jason Green <[EMAIL PROTECTED]> wrote:

The current cumulative patch is located here:

http://cmhousing.net/wine/glsl_cumul.diff

(This includes the recently submitted "Split constant loading out of
drawPrim()" patch, which hasn't been applied to git yet)

Before submitting, I plan to fix the following things:

- Fix relative addressing (it was working correctly, then I lost some
stuff when my hard drive got corrupted, and now it's using R0 instead
of A0 as the address register for some reason)
- Switch the shader_reg_maps.constantsF[] to a CHAR array instead of a
BOOL array (maybe even a bitmap(?))


This whole patchset should be a no-op for normal users, unless they
have the UseGLSL registry key enabled.  Here's a list of the
changelogs that this cumulative patch entails, somewhat in the order
that I'll be sending them:


[already sent] Move constant loading out of DrawPrimDrawStrided()
- DrawPrim is just too big of a function.  This separates the passing
of constants to the shader into new functions.
- Fixes an off-by-one error when loading vertex declaration constants
(should be <, not <=)
- Adds a function for GLSL loading of constants (aka Uniforms)
- Adds a GLSL program variable to the stateblock and sets it to 0 (a
future patch will actually create this program)

Add GLSL helper functions to device.c
- Adds functions to attach & detach shader objects, create and delete
programs, and maintain the list of programs.
- Adds a list of GLSL shader programs to the device which is
initialized on Init3D(), and deleted on Release()

Add the bulk of the GLSL string generation functions
- Add a new file glsl_shader.c which contains almost every GLSL
specific function we'll need
- Move print_glsl_info() into glsl_shader.c
- Move the shader_reg_maps struct info into the private header, and
make it part of SHADER_OPCODE_ARG.
- Create a new shared ps/vs register map for float constants (future
patch will make ARB programs use this, too)
- This is a big patch, but none of the new functions in glsl_shader.c
are being called yet.  This just sets them up.

Unified float constant register mapping between ARB pixel and vertex 
shaders.

- Got rid of the separate constant maps.
 - Side effect of this is that the map is a bit larger for pixel
shaders than it needs to be.
 - Will make this dynamic in a future patch.

Added more declarations to GLSL in generate_glsl_declarations()
- Declare more variable names for GLSL programs.
- Correct output name for pixel shaders (gl_FragColor instead of 
glFragColor)


Map pixel shader instructions to GLSL generating functions.
- Also, delete the GLSL program when the refcount hits 0

Map vertex shader instructions to GLSL generating functions.
- Also, delete the GLSL program when the refcount hits 0

Allow drawPrim to create and use the GLSL program
- Now that we can fully create a GLSL program, this patch lets us
actually use it.


I would have submitted these all separate for review, but I have some
janitorial git cleanup to do first to get all of this in the right
order.


- Nick






Re: wined3d: GLSL Patch feedback requested

2006-06-08 Thread Jason Green

By the way, here's a comparison screenshot of Civ4 from before my hard
drive failed, and this is without having texturing on pixel shaders
entirely working.  Using GLSL (working completely for vertex shaders
2.0, at least the instructions that Civilization 4 used):

http://cmhousing.net/wine/civ4_glsl.png

compared to:

http://cmhousing.net/wine/civ4_ingame2.png
and
http://cmhousing.net/wine/civ4_3.png

with current git and using regular ARB shaders.

So, we at least know that this is going to help.  :-)


On 6/8/06, Jason Green <[EMAIL PROTECTED]> wrote:

The current cumulative patch is located here:

http://cmhousing.net/wine/glsl_cumul.diff

(This includes the recently submitted "Split constant loading out of
drawPrim()" patch, which hasn't been applied to git yet)

Before submitting, I plan to fix the following things:

- Fix relative addressing (it was working correctly, then I lost some
stuff when my hard drive got corrupted, and now it's using R0 instead
of A0 as the address register for some reason)
- Switch the shader_reg_maps.constantsF[] to a CHAR array instead of a
BOOL array (maybe even a bitmap(?))


This whole patchset should be a no-op for normal users, unless they
have the UseGLSL registry key enabled.  Here's a list of the
changelogs that this cumulative patch entails, somewhat in the order
that I'll be sending them:


[already sent] Move constant loading out of DrawPrimDrawStrided()
- DrawPrim is just too big of a function.  This separates the passing
of constants to the shader into new functions.
- Fixes an off-by-one error when loading vertex declaration constants
(should be <, not <=)
- Adds a function for GLSL loading of constants (aka Uniforms)
- Adds a GLSL program variable to the stateblock and sets it to 0 (a
future patch will actually create this program)

Add GLSL helper functions to device.c
- Adds functions to attach & detach shader objects, create and delete
programs, and maintain the list of programs.
- Adds a list of GLSL shader programs to the device which is
initialized on Init3D(), and deleted on Release()

Add the bulk of the GLSL string generation functions
- Add a new file glsl_shader.c which contains almost every GLSL
specific function we'll need
- Move print_glsl_info() into glsl_shader.c
- Move the shader_reg_maps struct info into the private header, and
make it part of SHADER_OPCODE_ARG.
- Create a new shared ps/vs register map for float constants (future
patch will make ARB programs use this, too)
- This is a big patch, but none of the new functions in glsl_shader.c
are being called yet.  This just sets them up.

Unified float constant register mapping between ARB pixel and vertex shaders.
- Got rid of the separate constant maps.
 - Side effect of this is that the map is a bit larger for pixel
shaders than it needs to be.
 - Will make this dynamic in a future patch.

Added more declarations to GLSL in generate_glsl_declarations()
- Declare more variable names for GLSL programs.
- Correct output name for pixel shaders (gl_FragColor instead of glFragColor)

Map pixel shader instructions to GLSL generating functions.
- Also, delete the GLSL program when the refcount hits 0

Map vertex shader instructions to GLSL generating functions.
- Also, delete the GLSL program when the refcount hits 0

Allow drawPrim to create and use the GLSL program
- Now that we can fully create a GLSL program, this patch lets us
actually use it.


I would have submitted these all separate for review, but I have some
janitorial git cleanup to do first to get all of this in the right
order.






wined3d: GLSL Patch feedback requested

2006-06-08 Thread Jason Green

The current cumulative patch is located here:

http://cmhousing.net/wine/glsl_cumul.diff

(This includes the recently submitted "Split constant loading out of
drawPrim()" patch, which hasn't been applied to git yet)

Before submitting, I plan to fix the following things:

- Fix relative addressing (it was working correctly, then I lost some
stuff when my hard drive got corrupted, and now it's using R0 instead
of A0 as the address register for some reason)
- Switch the shader_reg_maps.constantsF[] to a CHAR array instead of a
BOOL array (maybe even a bitmap(?))


This whole patchset should be a no-op for normal users, unless they
have the UseGLSL registry key enabled.  Here's a list of the
changelogs that this cumulative patch entails, somewhat in the order
that I'll be sending them:


[already sent] Move constant loading out of DrawPrimDrawStrided()
- DrawPrim is just too big of a function.  This separates the passing
of constants to the shader into new functions.
- Fixes an off-by-one error when loading vertex declaration constants
(should be <, not <=)
- Adds a function for GLSL loading of constants (aka Uniforms)
- Adds a GLSL program variable to the stateblock and sets it to 0 (a
future patch will actually create this program)

Add GLSL helper functions to device.c
- Adds functions to attach & detach shader objects, create and delete
programs, and maintain the list of programs.
- Adds a list of GLSL shader programs to the device which is
initialized on Init3D(), and deleted on Release()

Add the bulk of the GLSL string generation functions
- Add a new file glsl_shader.c which contains almost every GLSL
specific function we'll need
- Move print_glsl_info() into glsl_shader.c
- Move the shader_reg_maps struct info into the private header, and
make it part of SHADER_OPCODE_ARG.
- Create a new shared ps/vs register map for float constants (future
patch will make ARB programs use this, too)
- This is a big patch, but none of the new functions in glsl_shader.c
are being called yet.  This just sets them up.

Unified float constant register mapping between ARB pixel and vertex shaders.
- Got rid of the separate constant maps.
- Side effect of this is that the map is a bit larger for pixel
shaders than it needs to be.
- Will make this dynamic in a future patch.

Added more declarations to GLSL in generate_glsl_declarations()
- Declare more variable names for GLSL programs.
- Correct output name for pixel shaders (gl_FragColor instead of glFragColor)

Map pixel shader instructions to GLSL generating functions.
- Also, delete the GLSL program when the refcount hits 0

Map vertex shader instructions to GLSL generating functions.
- Also, delete the GLSL program when the refcount hits 0

Allow drawPrim to create and use the GLSL program
- Now that we can fully create a GLSL program, this patch lets us
actually use it.


I would have submitted these all separate for review, but I have some
janitorial git cleanup to do first to get all of this in the right
order.