Please everybody test issue:
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/25124 Some components caused an error when placed on a Frame. Now it apparently works.
Can somebody please explain why it works. (See my comment there).

There are also improvements for a custom Frame installed as a component in the palette.
See:
https://gitlab.com/freepascal.org/lazarus/lazarus/-/issues/39488

The improvements will be merged to 2.2.

Regards,
Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/4edb56bd/attachment.html>

From reading the issue, and a peak at the code:

- When the frame is read from stream, the property is triggered, as it should (and as on a Form.
- The property accesses "canvas" => that triggers asking for a handle (which is created based on CreateParams)

A form does not need a parent (it can have one, if it is embedded). Therefore CreateParam for TForm checks this and removes flags such as WS_CHILD (not sure if WS_TABSTOP would cause an issue, but it makes no sense...)

A frame, at least at design time, can display without parent too. (I guess: it acts as if it is a TForm in that case).
=> So for that IMHO your patch is correct (for design time).

Yet a TFrame at runtime IMHO should always have a parent. No idea, if there is other code, that relies on that.... So maybe your patch should be extended, to only do that, if the control is in "designtime" state?
Though, not sure if it is worth it. While it would be correct to fail at runtime if there is no parent, it would not bring any benefit. If other code in TFrame needs the parent, then it will fail in the other code, if not then it will work.
The question is, do we commit to it may work, and then maybe have to fix more later? Otherwise it may want to even throw an exception, if it does not have a parent at runtime?

I often create a frame in code (so no parent) which is then placed on a form.

Will this scenario still work ?

Michael.

On Mon, Nov 29, 2021 at 12:36 PM Michael Van Canneyt via lazarus <

Ah yes, Canvas.
TStringGrid also uses Canvas a lot, but apparently not in a property setter.

I often create a frame in code (so no parent) which is then placed on a

form. Will this scenario still work ?

Yes, after my fix it still works.
It is better *not* to add other artificial restrictions. They would bring no benefit as Martin noted.

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/b9412dda/attachment.html>

What do you mean 'artificial restrictions' ? The above is quite standard.

Michael.

Well, if your code works now, then apparently it does not access the handle/canvas before setting a parent?
Which also means, you do not "show" the frame, before it has a parent.

1)
With Juha's current patch: Yes. => Afaik this adds no restrictions. => Except, if you are waiting for an Exception, because you expect it to fail. ;)

2)
If we add a "if designtime" around Juhas code, still nothing changes for your code. Since then runtime behaviour would be exactly as it currently is.

3)
If we add
   If (not designtime) and (ParentWindow = nil) then raise Exception(); afaik still nothing changes (except for the error message). Because currently, without Parent/ParentWindow it will fail to create the Handle (Actually, may need to be tested for all WidgetSets).

If your code creates a Frame that during its creation (reading from lfm, or calls by other code) triggers any component to access the canvas, or otherwise needing a handle,
then your code would fail already (without Juha's Patch).

With the current Patch, such an "canvas" access would create a handle (it might not make it visible, but it could even do that). If that handle is created, and you set a Parent, then the handle would have to be discarded.

So unless a frame should be able to be visible shown at runtime as if it was a Form of itself, any handle created without parent is created to be discarded again.
Such a handle would only serve the purpose to allow incorrect code to "get away" (I.e. to draw text on an invisible canvas, that will never become visible).

Question is:
- Should that be allowed?
- Should components access the canvas in "csLoading", or "if not HandleCreated"  (unless they indent for the Handle to be created at that time, and therefore are aware if that is possible)

Michael Van Canneyt wrote

What do you mean 'artificial restrictions' ?  The above is quite standard.

IMHO  "if csLoading" tests are also "quite standard"

"If HandleCreated" exist too, not sure how widespread....

"accessing canvas"  outside paint, is discouraged. Though other uses of the "Handle" may exist outside paint.

Yes, definitely add a condition to check the designtime state.

There are many scenarios when the Canvas cannot be accessed and it is a common mistake to access it when not allowed.

I didn't study the issue further but to me it looks strange that setting some parameters in CreateParams helps with it. Juha, your commit description "Somehow fixes issue ..." doesn't help to understand your change either.

So I would encourage the component author to rewrite his code.

Ondrej

Well, as I understood it, painting code will not work on Mac outside the paint event, so only using canvas inside paint should be standard ? Reading is of course possible, I suppose.

Michael.

You can still use the Canvas outside Paint for measuring. But not for painting. Painting outside Paint on Windows it is problematic as well.

The "use the Canvas outside Paint for measuring" is valid of course only as long as Canvas is there or can be created - and it can be created when the handle is created or can be created. And a (normal) control's handle can be created only when a parent is set.

That is the problem - the code tries to access canvas when the control's handle cannot be created.

Ondrej

Ondrej Pokorny wrote

I didn't study the issue further but to me it looks strange that setting some parameters in CreateParams helps with it. Juha, your commit description "Somehow fixes issue ..." doesn't help to understand your change either.

"Setting" is actually "removing" => removing "WS_Child" allows the frame to be created as a top-level window.

So before this, while the frame was created (and did not have a parent), that "buggy component" accessed the canvas. => Handle creation failed. => Now handle creation works. (note, that the handle is probably never "shown" , set to visible=true)

So that is why the patch "works"

Actually, come to think of it, maybe it be different. => it just allows a temporary handle (that will be thrown away without ever being visible), only to support buggy code, and that should not happen....

Yet:
- Afaik/IIRC if read from stream as part of a form in runtime => the parent is already set (the form).
  So at runtime such buggy code would work
- If indeed so, the IDE would have to make sure, that the "designer" is already set as a parent, when reading the stream.

Well, that is:
I guess, when the frame is show stand alone in the IDE it has the designer as parent (not verified, but must have something, as it can be shown -- and until now it could not be shown as top level window).

Then again, setting the designer as parent for reading the stream, may not be which case we may go with the "dummy handle" but for designtime only.

On Mon, Nov 29, 2021 at 1:32 PM Ondrej Pokorny via lazarus <

The commit message is not perfect but the committed code is, now that I fully understand the issue.
CreateParams for Frame now follows the same logic as CreateParams for Form. It allows a Frame to stand without a parent in the designer or even at runtime in some hypothetical situation(?).
Using Canvas outside Paint may not be recommended but it can be done in some widgetsets. The component's duty is to not crash in designer or anywhere else when it happens.

So I would encourage the component author to rewrite his code.

Rewrite? Ok, then TCustomForm must be rewritten, too. They now follow the same logic, at least regarding CreateParams().

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/713a3b89/attachment-0001.html>

I suspect in the Designer it does not stand alone => The designer is the parent. But I have not checked.

As for "at runtime" and "stand alone" (which at design time may not be possible, if the designer forces itself as parent). Is that now possible. Has anyone tested?
Just because it can get a handle does not mean it will work if you try to make the handle visible.

If it does fine.

If it does not, then we have a new problem.
We now provide a canvas, and let the user believe they painted on it. But its invisible. And if in order to make it visible, a parent is required, then that means a new Handle.
So then we throw away the users work. (and the user will forever wonder way his output is not drawn....)

So can a frame be visible shown standalone? And do we want to support that. (fix bugs on it, make it work for all widgetsets, ....) -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/6e71759b/attachment.html>

On Mon, Nov 29, 2021 at 6:47 PM Martin Frb via lazarus <

No, it has no parent. That is why it crashed.
This is with the default IDE with floating windows. With the docked IDE it does have a parent.

If it does not, then we have a new problem.

The same potential problem applies to TForm. They both behave equally well now in the designer.

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/c1ab54fc/attachment.html>

Frame is not a form.

It allows a Frame to stand without a parent in the designer or even at runtime in some hypothetical situation(?).

We definitely should't do anything with the parameters at runtime and don't allow frames to be shown as forms by default. What buttons (min/max/close/...) should be shown, what border style wtc etc? If the programmer wants to show his frame as a standalone form, he can override CreateParams() on his own.

Using Canvas outside Paint may not be recommended but it can be done in some widgetsets. The component's duty is to not crash in designer or anywhere else when it happens.

Well, if the programmer writes his code so that it crashes, what do you want to do with it?

Yes, we should handle exceptions in the designer. But your code doesn't improve this.

Rewrite his code = the custom component, not TFrame, not TCustomForm.

It's basically a very bad idea to force create the handle when the component is loaded (that is what you do when you access the canvas).

Ondrej
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/a7ee505a/attachment-0001.html>

Unless you have installed the docked designer package (I don't know in what shape it is now, AFAIR Michl has done some improvements), the designer form is standalone and the frame as well (=they don't have a parent).

I assume the IDE designer sets the needed window parameters for the frame at some place between loading and showing the frame. But I haven't checked now and it's been a while when I fiddled with the designer code the last time.

Ondrej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/041a1163/attachment.html>

Ondrej Pokorny wrote

It's basically a very bad idea to force create the handle when the component is loaded (that is what you do when you access the canvas).

Check TCustomLabel.CalculateSize in lcl\include\customlabel.inc for the solution how to solve this problem correctly in the component's code without any modifications needed in T(Custom)Form or T(Custom)Frame.

There, the GetDC(0) is used for Canvas.Handle, so that the label's parent's handle doesn't need to be created.

Ondrej

That raises another question: Why does it need that before/unless it is visible?

Does autosize run, for non-visible components?

That is nonsense. I reverted your change. The code user code is just plain wrong and your change in TFrame doesn't change anything about it.

Try e.g. :

procedure TForm1.Button1Click(Sender: TObject);
var
   grid: TNewGrid;
begin
   grid := TNewGrid.Create(Self);
   grid.MyProperty := 1; // exception
end;

Btw. Delphi behaves the same.

Ondrej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211129/9e5f4914/attachment.html>

On Mon, Nov 29, 2021 at 11:14 PM Ondrej Pokorny via lazarus <

No, the code is valid although not recommended. Accessing Canvas outside Paint works with some widgetsets and then an exception is wrong.

Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211130/cb40cd8f/attachment.html>

No, Juha, you don't understand the Canvas outside Paint is OK under all widgetsets if done correctly. Whenever you access the canvas for text measuring you have to make sure you can do so. There are 2 solutions for it:

1.) Check HandleAllocated and skip the measurements if the handle is not allocated. Like here:
https://gitlab.com/freepascal.org/lazarus/lazarus/-/commit/6e5e5b67df9d26cd477588a27029c8007d08add2

- or -

2.) If you really have to do the measurements and return something, create an extra handle for the canvas like in TCustomLabel.CalculateSize.

---

The LCL's function is not to force creating the canvas when it cannot be naturally created with manipulating itself like you did.

Ondrej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211130/45a84357/attachment.html>

There are 2 (or 3) question.

1) Is the fix correct: Yes (if 2 or 3), otherwise "not applicable"

2) Is it a bug in first? (and therefore is a fix needed): No. No (at least if it is at runtime) it is not a bug, because it is by design that a frame needs a form as parent.

3) Before the patch, ignoring the design time issues => did it work at runtime? (And is it indented to?)
When (without the patch) the stream is read at runtime, and the frame is embedded in a Form, does the example code then work? Basically, will at runtime the parent be set, before any properties are loaded?
(Or, Is that documented that it should?)

=> *If* this works at runtime (and if this is by intention), then there is a bug in the designer, in that the designer fails to set the parent (parent = designer) in time.

I have not tested the example from the bug. But I did a quick debug of a TFrame being loaded, at runtime:
- The TFrame is loading twice from lfm (the frames own lfm, and the changeds stored in the form)
-- the first loading, there is no parent set => so it would/could fail. -- the 2nd loading, there is a parent

The exact "expected" (not tested) behaviour for the app from the bug would be:
- If the offending property has a default, and is not changed from the default => all good, property is not set during TReader
- If the default is changed in the Frame's lfm => crash, because the property is set, when there is no parent.
- if the default is only changed for the frame's instance on the form (stored in the form's lfm) => ok

So IMHO it is save to say, that this is not supported at runtime either. Should it be? IMHO not, see below.

-----------------------
This leaves improving the designer, to add a better error message, and handle it more gracefully.

-----------------------

Otherwise Ondrej is right that code accessing the handle (and that includes canvas) must do checks.
And TFrame is not the only place were it this is needed.

   b := TPanel.Create(Form1);
   a := TGraphicControl.Create(b);
   //a.Parent := b;
   //b.Parent := Form1;
   a.Canvas.Line(1,1,2,2);

You must set both parents, or it crashes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211130/17c809f9/attachment.html>

On Mon, Nov 29, 2021 at 11:14 PM Ondrej Pokorny via lazarus <

Of course your example code throws an exception because the grid has no Parent. Assign a Parent after creation and it works. Your example shows that you don't understand the issue at all! The problem was not the grid's parent but the Frame's parent. The grid's parent is requested, it is there, then its parent's parent is requested. The Frame has no Parent by definition at design time -> exception.

My fix is 100% correct. Calling it nonsense and reverting it was not nice. Please restore it. You can fix the commit message at the same go. It must go to 2.2, too. One less bug in the release.

There is one alternative solution. If you are right and the component behaves wrong, then an exception must be thrown when it is placed on a Form as well.
Now the behavior is inconsistent and buggy.
I personally don't see why component authors should be punished with such an exception. If the component does not work, it will be evident by other means.

@Martin:

2) Is it a bug in first? (and therefore is a fix needed): No. No (at least if it is at runtime) it is not a bug, because it is by

design that a frame needs a form as parent.

Why you play dummy now? You know the problem happens at design time, not runtime.
The exception happened only because the code required a Frame to have a Parent which BY DEFINITION it does not have at design time under the default designer.
The correct fix is to NOT require a Frame to have a Parent, which I did in my commit.

3) Before the patch, ignoring the design time issues => did it work at

runtime? (And is it indented to?)

Yes, at runtime it works on both a Form and a Frame. At design time it works on a Form but crashes on a Frame which is clearly a bug. Agree?

I wrote about a hypothetical situation where a Frame stands alone at runtime. It obviously does not happen with the current TFrame. I meant that my fix is logically correct also if somebody derives a *SuperFrame* for whatever reason for extra capabilities. The frame's params would still be right then.

Yes, the error message is confusing.
"*NewGrid1.MyProperty: Control '' has no parent window*" while it actually came recursively from the Frame.Parent.
Maybe it confused Ondrej's head. It confused mine initially, and Flávio Etrusco's (see his comment).
Now that I understand the issue, my fix clearly was the right one.

Regards,
Juha
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211130/bf087052/attachment-0001.html>

Juha Manninen wrote

Yes, at runtime it works on both a Form and a Frame. At design time it works on a Form but crashes on a Frame which is clearly a bug. Agree?

No, at runtime it crashes too.

Of course, if you take the attached zip, you will not see this.

Because the Frame is not actually used in TForm1 => so at run time the frame is never loaded.

If you manage to get the frame on form1 then it crashes too. -------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lazarus-ide.org/pipermail/lazarus/attachments/20211130/51683f13/attachment.html>