Don't include core/io/image.h in core/os/os.h

[dustdfg]

core/os/os.h doesn't use core/io/image.h

It just brings to the code base transitive dependencies. Lots of dependencies because core/os/os.h is transitively included in almost every file of godot

How numbers were obtained. First of all thanks to this message in chat I knew about --tree=linedraw. So after compiling I run scons --tree=linedraw > /tmpt/godot.linedraw after that I needed to get actual number of includes of tested files. By running grep "<file>" /tmp/godot.linedraw | wc -l you get a number of files which directly or transitively depend on <file>

	core/os/os.h	core/io/image.h
Master	3040	3073
PR	3040	2650

Summary. With this PR compiler will need to process core/io/image.h 423 times less. Numbers were obtained on linux and will differ a little on different platforms because they doesn't share files inside platform folder.

It definitely should improve compilation speed but how much.... Maybe several microseconds, maybe several minutes. I can't check. One full build build from clean state takes 1.5~ hours on my machine so if you have a more powerful machine, could you please test if there is difference in compile times?

This PR also contains one minor change. core/core_bind.h didn't use core/io/image.h in any way as like all who includes it so this change doesn't depend core/os/os.h file

[fire]

Since this is a compiler update, if it passes Github actions cicd it's fine.

I don't expect a speedup but it's neat on a clarity point of view.

[AThousandShips]

Should be safe, only risk is obscure compile errors in classes using Ref<Image>, did you cover all such cases specifically? If so then this should be proof against future changes and the only real risk is existing PRs that would need to be adjusted

[dustdfg]

I honestly doesn't know anything about Ref<Image> and didn't saw compiler errors which involve Ref at all and don't know what you are talking about ¯\_(ツ)_/¯

[bruvzg]

This include is likely an old one from 3.x, a bunch of Ref<Image> function were in OS, but now are in DisplatyServer.

[AThousandShips]

The issue is explained here #80330 (comment)

[dustdfg]

Honestly I can't say that I really understand Ref problem. Is the problem applicable only for forward declared things?

I removed forward declaration of Image inside display_server.h because it already included os.h which included image.h so IIUC there couldn't be problem with that forward declaration because the full definition also was pulled through os.h. Now there is no forward declaration at all so I want to believe there shouldn't be problem you describe.

[AThousandShips]

Are you sure there are no places that used to include this that have Ref<Image> and don't include the header? If yes then there's no problem, otherwise it is a risk and should have an include in the header as a replacement

[dustdfg]

Are you sure there are no places that used to include this that have Ref<Image> and don't include the header? If yes then there's no problem, otherwise it is a risk and should have an include in the header as a replacement

It looks like there are some places where the potential problem exist. I am almost found an easy way to find them with grep and etc so I think I will solve them tomorrow. Thank you for explanations 😃

[dustdfg]

Are you sure there are no places that used to include this that have Ref<Image> and don't include the header? If yes then there's no problem, otherwise it is a risk and should have an include in the header as a replacement

Now I am sure. Though I didn't add it to the files which include core/io/image.h or core/io/image_loader.h directly inside .cpp or in corresponding .h file with the same name

[Repiteo]

Thanks!