[PATCH 1/2] windowscodecs: Use common base structure for component info implementations.
Signed-off-by: Jacek Caban jacek@codeweavers.com --- dlls/windowscodecs/info.c | 148 ++++++++++++++++++++++------------------------ 1 file changed, 71 insertions(+), 77 deletions(-)
This looks fine, but I'm not sure if incrementally changing our current implementation is the best approach. I know Windows doesn't store data in the registry at all for builtin components, and my impression is that for custom components it reads from the registry all at once.
I think the ideal for us is to have some internal data structure containing all the data, which would be static for builtins and dynamic for registry info. We could probably get rid of the custom registration for windowscodecs.dll on prefix creation (but I'm not sure about the COM stuff).
I'm not aware of any application that uses its own custom components, so it might not be all that bad to start over with static data and leave the dynamic data for later if we need it. But that would be removing a feature that theoretically works now.
Hi Vincent,
On 03/29/2018 10:11 PM, Vincent Povirk wrote:
This looks fine, but I'm not sure if incrementally changing our current implementation is the best approach.
Incremental changes have value of their own (easier to review, less likely to break, more regression testing friendly), but sure, depending on end goal might not be the best approach. I still think it is the right approach in this case.
I know Windows doesn't store data in the registry at all for builtin components,
Yeah, it would be nice to do that as well.
and my impression is that for custom components it reads from the registry all at once.
My vision for future of this is to read it all in component info constructor. Once we'd have that, a change between reading on the first CreateComponentInfo() for given component or all at once would be rather small and straightforward.
I think the ideal for us is to have some internal data structure containing all the data, which would be static for builtins and dynamic for registry info.
I'm not sure I agree. For example, the form in which GetPattern() returns pattern is not cleanly expressed as C constants (and regsvr.c has it in a form that allows clean C constants). I guess we need the code to do the conversion, the question is where do we want it. I'd say it's better to convert that in constructor and do simple memcpy() inside GetPattern() instead of doing the conversion every time GetPattern() is called. If we do it that way, registry-based component info could fill that structure inside constructor as well and GetPattern() wouldn't need to care if it's builtin or custom component.
We could probably get rid of the custom registration for windowscodecs.dll on prefix creation
That would be nice.
(but I'm not sure about the COM stuff).
We can't get rid of COM registration, but note that once the registration is purely .rgs file based and doesn't need any thing else, Wine is smart and uses that .rgs resources directly instead of calling DllRegisterServer so it's much faster.
I'm not aware of any application that uses its own custom components, so it might not be all that bad to start over with static data and leave the dynamic data for later if we need it. But that would be removing a feature that theoretically works now.
I don't think that a step forward.
Thanks,
Jacek
-
Jacek Caban -
Vincent Povirk