issue-#106 View restored twice on Open

[Josef Templ]

I call it an optimization.
The docu states clearly that there must not be any assumptions on the number of Restore operations.

Anyway, such a change is too late for 1.7 because there is no time for testing the change.
Testing the change extensively is important because such a change has a lot of
potential for unwanted side effects.
1.7 is not the end of the world.
If you or anybody else finds a fix it can of course be considered,
but not as a last minute change of 1.7.

- Josef

[Ivan Denisov]

I added 1.8 to tracker.
So now we can change issue target version to 1.8.
http://redmine.blackboxframework.org/issues/106

[DGDanforth]

OK, that is fine with me to move it to 1.8.
-Doug

[luowy]

Restore twice on Open results from:

HostWindows.OpenDoc
(1) creates a child window, which is restored, and
(2) the child window is updated, which also means that it is restored.

By commenting w.Update in HostWindows.CreateDoc (the restore in the first step)
you can avoid restoring twice on open.
HOWEVER, there are strange things then in some views, in particular
the scrollbars are not set up correctly.
So the first Update has side effects that are important.

Making any changes in the opening of documents goes very deep into
the framework and is probably wrong in some unexpected sense.
I would strongly recommend not to touch this code until we have a much
better understanding of how it works.

- Josef

@Josef,
can you give a detail(demo) about "the scrollbars are not set up correctly."?
I cant find it after "commenting w.Update in HostWindows.CreateDoc".

[Josef Templ]

For example, open System/Mod/Kernel.odc.
With "w.Update" commented in HostWindows.CreateDoc you get a horizontal scroll bar,
which you didn't get before.

- Josef

[luowy]

For example, open System/Mod/Kernel.odc.
With "w.Update" commented in HostWindows.CreateDoc you get a horizontal scroll bar,
which you didn't get before.

- Josef

comment out this line together

Code: Select all

IF ~w.fix THEN w.SetSize(cw, ch) END;

[Josef Templ]

Interesting observation but there are still some open questions.

Why didn't you also comment out the line in between ("w.UpdateScrollbars(FALSE);")?
Why is some of the commented code in bold face?
Is this the last unwanted side effect?
Is there any explanation of the underlying behavior?

- Josef

[DGDanforth]

The docu states clearly that there must not be any assumptions on the number of Restore operations.
- Josef

In my opinion that is a poor design.
The framework needs to be redesigned so that an "Open" calls "Restore" only once.
-Doug

[Robert]

I am leaving this Issue with the status Dormant because, as far as I am aware, the following four conditions apply:

→ The proposed change to the behaviour has not been rejected as undesirable
→ It has been open for a long time
→ There is no practical proposed implementation available
→ No one is actively working to find such an implementation.

Doug, maybe a way to move this forward is to write privately to Oms and see if they have any insight into why the current design, with two restores, was chosen?

[Robert]

Doug
Maybe you could try this workaround, and report if it helps you, or causes any problems?

Add the following three (harmless ?) lines to HostWindows:
→ In the global VAR section "src-: INTEGER;"
→ In the procedure CreateDoc (the first line in this clip is new)

Code: Select all

src := 1;
		w.Update;
		w.UpdateScrollbars(FALSE);
		IF ~w.fix THEN w.SetSize(cw, ch) END;
		res := WinApi.GetWindowRect(wnd, rect);
		w.mw := rect.right - rect.left; w.mh := rect.bottom - rect.top;
		HostMechanisms.InstallDropTarget(wnd, w);
		w.setup := TRUE
	END CreateDoc;

→ In the procedure PaintWin

Code: Select all

	PROCEDURE PaintWin (wnd: WinApi.HANDLE; a, b: INTEGER);
		VAR w: Window;
	BEGIN
		w := ThisWindow(wnd);
src := 2;
		w.Update
	END PaintWin;

Now, in your View's Restore procedure, add the test:

Code: Select all

    IF  HostWindows.src = 2  THEN
      f.Draw...
    END;