TCursorImageImage is created.
No it is not created as a TCursorImageImage, memory is allocated using the correct instance size, but TCursorImageImage.Create(const AInfo: TIconInfo);, line 2817 of graphics.pp is never called, you can put a break point on it. So the behaviour is pretty indeterminable, it is just an additional class that does not bring any improvement (but adds one more class with the compiler generated code block and nothing really useful).
Ah, I missed that info. Though I strongly disagree with the wording.
There is a difference between "a class being created" and "a (specific) contructor being (not) called".
A class can be created, even if it has no constructor of its own (TObject.create is always the fallback). Yet the class is created.
So therefore TCursorImageImage is created.
The base class TIconImage has 3 different constructors to choose from.
1)
2 of them take arguments (TPixelFormat, TRawImage) that have no Hotspot info. So the constructor in TCursorImageImage has nothing to do in that case.
2)
2 of them are not virtual, and do not call the virtual one.
That indeed indicates a serious design issue.Generally the concept of virtual constructors, and offering multiple constructors (that do not call each other), is a problem too. As it means the child class will have to override them all.
There should be one constructor, that is called by all of them. Or alternatively an "Init; virtual" method.
So yes that does need further review.
As for how to fix, I can not tell yet. For that the original intent has to be deducted (svn blame)
TCursorImage should be removed and the HotSpot fields should be stored, either in fields to add to TIconImage or, more consistently, since we deal with the hotspot of a cursor, in TCursorImage.
TCursorImage or TCursorImageImage? I assume the latter?
Can't comment until further research.
But the instance is stored on FImages, suggesting there could be more than one (not sure that can happen for TCursorImage?). If that can happen, then there is more than one Hotspot.
Also need to review towards backward compatibility
The other changes I made was removing Inherited for some classes that didn't need them. Every time inherited is not uselessly called saves some hundreds of cpu cycles, constructor create is quite fat. (Trying to fight against so said Witrh's law https://en.wikipedia.org/wiki/Wirth%27s_law)
Need to check that. Partly its a general design decision. It all comes down, if there is a chance that ever anything could be added into the now empty constructor.
---------
I am not sure when or if I get to dive into those details (hence the request to put all of it on the bugtracker).
Different people on the team, look after different parts of the code. (And occasionally step in, outside there main area). My main area is SynEdit and the debugger(s).