renamed macro FLUTTER_PLUGIN_EXPORT

[technolion]

This fixes #3

[bitsdojo]

Thank you, Thomas!

[stuartmorgan]

Is there a reason you removed the #ifdefs around that macro in the first place? Unless I'm not understanding the error, this only happened because you removed code that was in the template (and was there for a reason).

[bitsdojo]

Hi Stuart,

Thank you for making me take a second look at this.

Actually, I could remove that macro altogether and the usage in the BitsdojoWindowPluginRegisterWithRegistrar function definition as I see no need for it. The package compiles as a static library so I believe there is no need for __declspec(dllexport) or __declspec(dllimport).

Do you see any reason for keeping that macro? Open to any suggestions you might have regarding this.

[stuartmorgan]

The package compiles as a static library

Can I ask why you made that change? The Windows plugin system is designed for shared libraries; what you are doing is not supported, and could easily break in the future, or in conjunction with other plugins (especially if they made the same unsupported change).

[bitsdojo]

Can I ask why you made that change?

I need to run code that handles specific window messages very early in the window creation process (e.g. WM_NCCALCSIZE) and to make sure that the window is hidden on startup and I could not find any way to do it from a shared library.

Shared libraries plugins are loaded way too late to handle those messages and the only option was to create a static library that can run code when the executable is started.

Do you know any way to achieve the same from a shared library? If so, I would be happy to modify the package.

what you are doing is not supported, and could easily break in the future, or in conjunction with other plugins

I would love to know your thoughts on this. Do you have any specific situation in mind?

[stuartmorgan]

I need to run code that handles specific window messages very early in the window creation process

I see. I hadn't looked at the details of what you were doing.

Hopefully you are aware that essentially everything you are doing here is unsupported; this is not so much a plugin as something that modifies the runner's behavior that is (ab)using the plugin infrastructure to get itself loaded. There's no guarantee that much of what you are doing here won't break in the future.

The right way to change window creation itself is, currently, to change code in the runner. Hooking it using static initialization instead is, IMO, a hack, and is fragile. (As one of many possible examples: there's no reason that the window class name couldn't change in the future. Or even that someone making an app couldn't change their own window class name.) For non creation-time things, there is an API to register window message handlers that's official and supported, and I'm not sure why you aren't using it.

This is certainly interesting as a proof of concept, but publishing it as a plugin without any caveats that it's unsupported seems like it has a high potential for future issues and confusion.

Do you have any specific situation in mind?

This issue was one; no other plugin has the same collision problem. I've mentioned another already. Another possibility is a one-definition-rule violation, which causes undefined behavior for the entire application. I doubt that's an exhaustive list.

[technolion]

Devs will love this as a plugin, as it very much improves the appearance of a flutter app on windows. Although I agree, that this functionality should be included in the flutter framework itself, it should be okay to have it within a plugin until that happens.
Maybe the fact that this plugin's approach could break in the future should be documented when publishing it on pub.dev. Is there something like an experimental flag on pub.dev?

People really like to get going with Flutter on Desktop and this plugin helps to fix one of the remaining key issues (like Drag & Drop across application boundaries) that's urgently needed.

[bitsdojo]

The right way to change window creation itself is, currently, to change code in the runner. Hooking it using static initialization instead is, IMO, a hack, and is fragile.

I agree with you, it would be ideal to change code in the runner to do this. However, we must keep in mind that not every Flutter developer is a Win32 developer and forcing them to learn about window messages to modify runner code would be, I believe, harder than using a package like this. This is the reason why this package was created:

I was getting tired of modifying the runner code to add window message handling for every new desktop app I wanted to create with Flutter. So I created this package as an easier way to do this for every app.

(As one of many possible examples: there's no reason that the window class name couldn't change in the future.

This could be easily fixed with a future update and I believe there is a low probability that this would change so frequently in order for it to be considered an issue.

Or even that someone making an app couldn't change their own window class name.)

Someone changing their own window class name probably has enough win32 knowledge to know what they are doing and probably wouldn't need this package in the first place.

For non creation-time things, there is an API to register window message handlers that's official and supported, and I'm not sure why you aren't using it.

I used it initially when starting to work on this package but I later dropped it as it didn't help with messages sent before WM_CREATE.

Hopefully you are aware that essentially everything you are doing here is unsupported; this is not so much a plugin as something that modifies the runner's behavior that is (ab)using the plugin infrastructure to get itself loaded.

The current plugin system in Flutter does not offer a way to achieve this without linking it as a static library so yes, you can say it is unsupported for that reason. Plugins are only loaded during WM_CREATE so the functionality provided by this package is not available when building it as a shared library.

That's why the package was built as a static library. There is currently no other way I know of to achieve this functionality, and the official API to register window message handlers doesn't help with this.

I believe that currently this is the easiest way for a Flutter dev to achieve this functionality in their app without having to learn about Windows messages and modifying the runner code to add their custom handlers.

An ideal way would be for Flutter to offer a mechanism for plugins to run their code before the window is created. If that would happen in the future then this package could be built as a shared library. Until then, a static library remains the only option to achieve this without forcing users to learn win32 message handling to modify the runner code.

[stuartmorgan]

The current plugin system in Flutter does not offer a way to achieve this without linking it as a static library so yes, you can say it is unsupported for that reason.

I'm not clear on why you can't build as a DLL and export a function that your README tells people to call in wWinMain before window creation. That would make the plugin significantly less non-standard, and thus more future-proof, at the cost of needing to add a single line in the runner (which you're already doing anyway).

Plugins are only loaded during WM_CREATE

I'm not sure what this means. The plugins DLLs aren't being loaded manually by anything in Flutter, they are specified at build time. Their registration method is called during the window bringup, but that doesn't mean you couldn't call other exported methods earlier.

An ideal way would be for Flutter to offer a mechanism for plugins to run their code before the window is created.

A lot of things will change when multi-window is supported.

Until then, a static library remains the only option to achieve this without forcing users to learn win32 message handling to modify the runner code.

What I described above doesn't require users to know anything more than how to add a single line of generic C++ code to one file; I would be interested to know why that doesn't work.

[bitsdojo]

That would make the plugin significantly less non-standard, and thus more future-proof, at the cost of needing to add a single line in the runner (which you're already doing anyway).

Where would you add that line?

[stuartmorgan]

In wWinMain, sometime before the call to window.CreateAndShow.

Am I not understanding the way the hook registration works? I would assume you just need call it before the window creation happens. In which case an explicit call, rather than relying on static initialization as you are currently doing, seems like it would be much more robust.

[bitsdojo]

In wWinMain, sometime before the call to window.CreateAndShow.

That's how it worked initially. During some early sharing with other Flutter devs not familiar with how win32 works, I found out that simple instructions like:

"Add these two lines at the beginning of this file" works better than:

"In wWinMain, sometime before the call to window.CreateAndShow."

and is also more future proof in case the runner code template changes and that CreateAndShow function is renamed to something else or included in another function.

Another thing to take into consideration is performance. This is code that runs when the application is starting and you want that window to appear on screen as soon as possible. Wasting time with dynamic linking would add some extra time that would make that window appear later on screen.

In case you have a plugin that exposes functionality that will be used on demand, based on user interaction then sure, I would also prefer dynamic linking. However, here we have a piece of code that will always run when the app starts and you want that code to run as fast as possible. That's when I choose static linking.

[stuartmorgan]

I found out that simple instructions like:

"Add these two lines at the beginning of this file" works better than:

"In wWinMain, sometime before the call to window.CreateAndShow."

You could tell them to make it the first line of wWinMain if that's a concern.

and is also more future proof in case the runner code template changes

If you ignore the fact that building a plugin as a static library on Windows is not supported, and is subject to breakage at any time.

Another thing to take into consideration is performance. This is code that runs when the application is starting and you want that window to appear on screen as soon as possible. Wasting time with dynamic linking would add some extra time that would make that window appear later on screen.

The whole Flutter engine is dynamically linked, so I'm doubtful that this is going to make an appreciable difference.

Also, static initialization is well known to slow startup, so the fact that you're using it in multiple places as your bootstrapping method suggests you aren't actually optimizing for performance.

In case you have a plugin that exposes functionality that will be used on demand, based on user interaction then sure, I would also prefer dynamic linking. However, here we have a piece of code that will always run when the app starts and you want that code to run as fast as possible. That's when I choose static linking.

Again though, the situation is not that you have two supported options and chose one; you are explicitly changing away from the only supported option to do something that we do not guarantee will actually work. As long as you continue to use static linking your plugin will always be subject to breakage at any time, without warning.

I think you're also underestimating the substantial issues that may arise over time if other developers decide to copy your approach of changing the library type. The possibility of ODR violations that only happen when using a specific combination of plugins, for instance.

It doesn't seem like we're going to agree here though, so I'll bow out at this point.

[bitsdojo]

Also, static initialization is well known to slow startup

The discussion here is about static linking not static initialization. Those are two different things: static initialization != static linking

It doesn't seem like we're going to agree here though, so I'll bow out at this point.

Sure, trying to come to an agreement when not mentioning the same topic is hard (see example above) so we can agree to disagree.

[stuartmorgan]

The discussion here is about static linking not static initialization. Those are two different things: static initialization != static linking

I'm well aware of that. My point was simply that arguing that static linking is important to not slow startup seems odd to me given that in several places in the plugin you are using static initialization rather than an explicit call to bootstrap your hooks. I would not expect someone trying to micro-optimize startup to use something that's a startup-optimization anti-pattern.

My other comments were clearly about static linking, but 🤷

[bitsdojo]

My point was simply that arguing that static linking is important to not slow startup seems odd to me given that in several places in the plugin you are using static initialization rather than an explicit call to bootstrap your hooks. I would not expect someone trying to micro-optimize startup to use something that's a startup-optimization anti-pattern.

If you can point a specific example of static initialization in this repository that slows down the startup time, I would be happy to see it. Without an example, this discussion is irelevant.

My other comments were clearly about static linking, but 🤷

Comments like this one?

I think you're also underestimating the substantial issues that may arise over time if other developers decide to copy your approach of changing the library type. The possibility of ODR violations that only happen when using a specific combination of plugins, for instance.

Every developer is responsible for their work. One should only do this if they know what they are doing. Again, using a static library is useful when that code needs to run early at startup (e.g: enhancing window creation like this repository does).

There are lots of things happening when a DLL is loaded. Those are related to different actors outside of your application (AV/security solutions) and they can all influence startup time.

One should not go blindly and simply use a dynamic library or static library just because everyone else is doing it. I believe one should choose the right tool for the specific job they need to get done and not try to come up with generic and vague arguments to support a point of view.

If you have a specific suggestion for an improvement in this package then I am happy to discuss it.

Other than that, vaguely mentioning potential situations with low possibility of happening in the future or making vague comments about the code in this repository without pointing something specific, don’t really help anyone.

You and I both have more important things to take care of in our lives. Let’s not waste it on unproductive discussions.

I appreciate you taking the time to reply here and hope that we can have more productive discussions in the future.