Hello wine-devel,
I'm running ArchLinux with gcc 7.2.0.
Since commit e99811aee2dff88c41d6e2875401810a85325839 wine fails to build with
> ../../../wine-git/dlls/kernel32/module.c: In function ‘load_library’:
> ../../../wine-git/dlls/kernel32/module.c:987:44: error: initializer element
is not constant
> static const DWORD unsupported_flags = load_library_search_flags |
Minimal test-program to show the error, just use something like "gcc main.c"
to compile.
----------
static const int load_library_search_flags = 0;
int main()
{
static const int unsupported_flags = load_library_search_flags;
return 0;
}
----------
Looks like an compiler error, but I'm not sure. Mind looking into that?
Regards,
Fabian Maurer
On 29.08.2017 20:06, Fabian Maurer wrote:
> Fixes Bug 43632.
> winecfg used the broken behavior that was fixed
> in f7f7b89e2e9117811c91269643868c6d063db5e9,
> so we need to adjust it.
>
> Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
> ---
> programs/winecfg/winecfg.rc | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
Interesting thing to test I think is to verify if current cross-built
winecfg shows the same issue on Windows, before changing them all.
Fabian Maurer <dark.shadow4(a)web.de> writes:
> Fixes Bug 43632.
> winecfg used the broken behavior that was fixed
> in f7f7b89e2e9117811c91269643868c6d063db5e9,
> so we need to adjust it.
>
> Signed-off-by: Fabian Maurer <dark.shadow4(a)web.de>
> ---
> programs/winecfg/winecfg.rc | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
There are resources in other modules that look like they would have the
same problem, a grep over the codebase would probably be useful.
--
Alexandre Julliard
julliard(a)winehq.org
Hi.
I was investigating a regression that cause the CheckDeviceFormat function to return WINED3DERR_NOTAVAILABLE when quering for RESZ surfaces. (WINED3DFMT_RESZ). This is causing OblivionReloaded (that worked properly until wine 2.5) to not work properly (it detect an incompatible hardware in wine or NVIDIA in wine-staging).
I tacked the regression to this commit : https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…
[https://camo.githubusercontent.com/db3db2bf09d082a1f4244859890e83954d8410ab…]<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…>
wined3d: Allow all formats with "glInternal" set in CheckSurfaceCapab… · wine-mirror/wine@8c98be4<https://github.com/wine-mirror/wine/commit/8c98be4791f18f31b04a4a0f08d18979…>
github.com
…ility(). Rendertarget, depth/stencil and texturing restrictions are applied later in wined3d_check_device_format(), so to the extent this make a difference it mostly affects off-screen plain surf...
I tried to fix this issue.
The last patch I made in my local branch is adding a wined3d_format_texture_info in the format_texture_info array:
+ {WINED3DFMT_RESZ, GL_DEPTH24_STENCIL8, GL_DEPTH24_STENCIL8, 0,
+ GL_DEPTH_STENCIL, GL_UNSIGNED_INT, 0,
+ WINED3DFMT_FLAG_TEXTURE | WINED3DFMT_FLAG_DEPTH | WINED3DFMT_FLAG_STENCIL | WINED3DFMT_FLAG_RENDERTARGET | WINED3DFMT_FLAG_FBO_ATTACHABLE,
+ ARB_FRAMEBUFFER_OBJECT, NULL},
However I know almost nothing about the internal work of wined3d and opengl and don't know if this kind of addition is correct. OblivionReloaded works and can properly use the RESZ depth texture.
I based this patch on the INTZ and RAWZ surfaces that are somewhat reported to be similar in scope in some docs.
Also I don't know if the flags are correct or even meaningful in this scope. I could not find any documentation about the internal meaning of these flags. As the good Jon Snow I know nothing.
Note I never did a patch in wine before (except adding some debug message). So please tell me if I'm in a completly wrong direction.
Also note that I didn't run the test suite. Using make test is failing hard on an advapi32 test even without this patch applied.
I attached the full patch file.
Andrew Eikum <aeikum(a)codeweavers.com> writes:
> This fixes maximizing to the wrong monitor on a Mac with multiple
> monitors. Maximizing with multiple monitors continues to work correctly
> on Linux for me.
>
> Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com>
> ---
> dlls/user32/winpos.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
I'm not sure it makes sense to use only one coordinate of the monitor
rectangle. What are the coordinate values that trigger this?
--
Alexandre Julliard
julliard(a)winehq.org
2017-08-10 12:02 GMT+02:00 Paul Gofman <gofmanp(a)gmail.com>:
> Signed-off-by: Paul Gofman <gofmanp(a)gmail.com>
The patch is fine overall, I only have a few questions / nitpicks.
I'll skip the part about the delay with the review because it's old
news by now... :/
> ---
> dlls/d3dx9_36/d3dx9_private.h | 3 +-
> dlls/d3dx9_36/preshader.c | 312 ++++++++++++++++++------------------------
> 2 files changed, 135 insertions(+), 180 deletions(-)
>
> diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h
> index 2727463a18..5ac5e63aeb 100644
> --- a/dlls/d3dx9_36/d3dx9_private.h
> +++ b/dlls/d3dx9_36/d3dx9_private.h
> @@ -230,7 +230,7 @@ enum pres_reg_tables
> struct d3dx_const_param_eval_output
> {
> struct d3dx_parameter *param;
> - unsigned int table;
> + enum pres_reg_tables table;
The change is good but (unrelated to this patch) the enum type should
really use the singular form.
> static void dump_bytecode(void *data, unsigned int size)
> @@ -674,6 +614,40 @@ static HRESULT append_const_set(struct d3dx_const_tab *const_tab, struct d3dx_co
> return D3D_OK;
> }
>
> +static void append_pres_const_set(struct d3dx_const_tab *const_tab, struct d3dx_preshader *pres)
Nitpick: this function appends all the struct
d3dx_const_param_eval_output from the preshader so calling it
"append_pres_const_sets" (or even something more explicit to that
effect, e.g. clarifying that you're appending entries about the
preshader outputs, which are shader inputs) would be preferable.
> +{
> + unsigned int i;
> + struct d3dx_const_param_eval_output const_set;
> +
> + memset(&const_set, 0, sizeof(const_set));
You could initialize const_set in the definition instead, like:
struct d3dx_const_param_eval_output const_set = {NULL};
which looks slightly better to me. It doesn't matter much of course.
> @@ -971,10 +946,30 @@ static HRESULT get_constants_desc(unsigned int *byte_code, struct d3dx_const_tab
> if (FAILED(hr = init_set_constants_param(out, ctab, hc, inputs_param[index])))
> goto cleanup;
> }
> + if (pres)
> + append_pres_const_set(out, pres);
> if (out->const_set_count)
> {
> struct d3dx_const_param_eval_output *new_alloc;
>
> + qsort(out->const_set, out->const_set_count, sizeof(*out->const_set), compare_const_set);
> + for (i = 0; i < out->const_set_count - 1; ++i)
> + {
> + if (out->const_set[i].constant_class == D3DXPC_FORCE_DWORD
> + && out->const_set[i + 1].constant_class == D3DXPC_FORCE_DWORD
> + && out->const_set[i].table == out->const_set[i + 1].table
> + && out->const_set[i].register_index + out->const_set[i].register_count
> + >= out->const_set[i + 1].register_index)
> + {
> + out->const_set[i].register_count = out->const_set[i + 1].register_index
> + + out->const_set[i + 1].register_count - out->const_set[i].register_index;
I think this might lose account of some registers. E.g. consider the
case where const_set[i] has register_index == 0 and register_count ==
4 while const_set[i + 1] has register_index == 1 and register_count ==
1. This would seem to lose track of the last two components.
I know that preshaders found in the wild seem not to overwrite already
written output registers but I wouldn't feel safe with (silently)
depending on it.
> + memmove(&out->const_set[i + 1], &out->const_set[i + 2], sizeof(out->const_set[i])
> + * (out->const_set_count - i - 2));
> + --out->const_set_count;
> + --i;
Mostly matter of personal preference but rather than decrementing the
for counter in the if when coalescing I'd write this as a while and
increment "i" only in the (currently absent) else case.
> @@ -1529,22 +1534,56 @@ static void set_constants(struct d3dx_regstore *rs, struct d3dx_const_tab *const
> data += param->rows * param->columns;
> }
> start_offset = get_offset_reg(table, const_set->register_index);
> - if (table_info[table].type == param_type)
> - regstore_set_modified(rs, table, start_offset,
> - get_offset_reg(table, const_set->register_count) * const_set->element_count);
> - else
> + if (table_info[table].type != param_type)
> regstore_set_data(rs, table, start_offset, (unsigned int *)rs->tables[table] + start_offset,
> get_offset_reg(table, const_set->register_count) * const_set->element_count, param_type);
> }
> +
> +
> const_tab->update_version = new_update_version;
One blank line too many.
> -}
> + if (!update_device)
> + return D3D_OK;
>
> + for (const_idx = 0; const_idx < const_tab->const_set_count; ++const_idx)
> + {
> + struct d3dx_const_param_eval_output *const_set = &const_tab->const_set[const_idx];
> +
> + if (device_update_all || (const_set->param
> + ? is_param_dirty(const_set->param, update_version) : pres_dirty))
> + {
> + enum pres_reg_tables table = const_set->table;
> +
> + if (table == device_table && device_start + device_count == const_set->register_index)
Again nitpicking: are there better names for those variables? Maybe
use the "current_" prefix instead? It's pretty clear what's going on
either way, so up to you.