Page 1 of 2

Optimalization help!!!

PostPosted: August 28th, 2003, 8:39 pm
by Johnny_Bit
Code: Select all
procedure TMain.UpdateInf;
var
  EditorExist, TempValue: Boolean;
  I: Word;
  L: Integer;
  CurrEdit: THMemo;
begin
  CurrEdit:=CurrentEditor;
  if State=hsReady then
    begin
      EditorExist:=(CurrEdit <> nil);
      if EditorExist then
        begin
          with Main do
            begin
              ActiveControl:=CurrentEditor;
              Caption:='HTEL - ['+pcFiles.ActivePage.Caption+']';
              if ClipBoardPreview<>nil then
                ClipBoardPreview.cvClipboardPreview.Font:=CurrEdit.Font;
            end;
          if pcFiles.PageCount=1 then
            pcFiles.HideAllTabs:=True
          else
            begin
              pcFiles.HideAllTabs:=False;
              pcFiles.TabStop:=True;
            end;
        end
      else
        begin
          Main.Caption:=Application.Title;
        end;
//--Actions-Update--------------------------------------------------------------
    //File Actions
      if EditorExist then
        TempValue:=CurrEdit.Modified
      else
        TempValue:=False;
      actFileSave.Enabled:=EditorExist and TempValue;
      actFileSaveAs.Enabled:=EditorExist;
      if EditorExist then
        begin
          for I:=0 to pcFiles.PageCount-1 do
            begin
              TempValue:=TempValue or (pcFiles.Pages[I].Controls[0] as THMemo).Modified
            end;
        end
      else
        TempValue:=True;
      actFileSaveAll.Enabled:=EditorExist and TempValue;
      actFileClose.Enabled:=EditorExist;
      actFileCloseOthers.Enabled:=EditorExist and (pcFiles.PageCount>1);
      actFileCloseAll.Enabled:=EditorExist and (pcFiles.PageCount>1);
      actFileSendMail.Enabled:=EditorExist;
 { TODO 1 -oJohnny_Bit -cActions : Trza jeszcze popracować przy actFilePrint i actFilePrintPreviev }
      actFilePrintPreview.Enabled:=EditorExist and (Printer.Printers.Count>0);
    //Edit Actions
      if EditorExist then
        TempValue:=CurrEdit.CanUndo
      else
        TempValue:=False;
      actEditUndo.Enabled:=EditorExist and TempValue;
      if EditorExist then
        TempValue:=CurrEdit.SelLength>0
      else
        TempValue:=False;
      actEditCut.Enabled:=EditorExist and TempValue;
      actEditCopy.Enabled:=EditorExist and TempValue;
      actEditPaste.Enabled:=EditorExist and (Clipboard.HasFormat(CF_TEXT));
      actEditDelete.Enabled:=EditorExist and TempValue;
      if EditorExist then
        TempValue:=Length(CurrEdit.Text)>0
      else
        TempValue:=False;
      actEditSelectAll.Enabled:=EditorExist and TempValue;
      actEditFont.Enabled:=EditorExist;
      actEditWordWrap.Enabled:=EditorExist;
      if EditorExist then
        TempValue:=CurrEdit.WordWrap
      else
        TempValue:=False;
      actEditWordWrap.Checked:=TempValue;
      actEditUpperCase.Enabled:=EditorExist;
      actEditLowerCase.Enabled:=EditorExist;
      actEditFirstsUpperCase.Enabled:=EditorExist;
      actEditWin1250.Enabled:=EditorExist;
      actEditISO_8859_2.Enabled:=EditorExist;
    //Search Actions
      if EditorExist then
        TempValue:=Length(CurrEdit.Text)>0
      else
        TempValue:=False;
      actSearchFind.Enabled:=EditorExist and TempValue;
      actSearchFindNext.Enabled:=actSearchFind.Enabled and (fdFind.FindText<>'');
      actSearchReplace.Enabled:=EditorExist and TempValue;
    //Insert Actions
      actInsertStdDate.Enabled:=EditorExist;
      actInsertStdTime.Enabled:=EditorExist;
      actInsertStdDateTime.Enabled:=EditorExist;
      actInsertUsrDate.Enabled:=EditorExist;
      actInsertUsrTime.Enabled:=EditorExist;
      actInsertUsrDateTime.Enabled:=EditorExist;
      actInsertInFormat.Enabled:=EditorExist;
      actInsertFromFile.Enabled:=EditorExist;
    //Viev Actions
      sbStatus.Visible:=EditorExist;
      actViewStatusBar.Checked:=sbStatus.Visible;
      actViewStatusBar.Enabled:=EditorExist;
      cbTopBar.Visible:=tlbrToolbar.ButtonCount>0;
    //Help Actions
      {No Help Actions enabel/disable}
//--StatusBar-Update------------------------------------------------------------
      with sbStatus do
        if Visible then //Little optimalization hehe
          begin
            for I:=0 to Panels.Count-1 do
              Panels[I].Text:='';
            if EditorExist then
              Panels[0].ImageIndex:=pcFiles.ActivePage.ImageIndex;
              Panels[1].Text:=Format('Col: %d; Row: %d', [CurrEdit.CaretPos.X, CurrEdit.CaretPos.Y]);
              L:=Length(CurrEdit.Text);
              case L of
                0..1024: Panels[2].Text:=Format('%d B', [L]);
                1025..1048576: Panels[2].Text:=Format('%3.2f KB', [L/1024])
                else
                  Panels[2].Text:=Format('%3.2f MB', [L/1048576]);
              end;
            Panels[4].MinWidth:=Canvas.TextWidth('INS')+10;
            Panels[4].MaxWidth:=Canvas.TextWidth('INS')+10;
            case ksKeys.Insert of
              True : Panels[4].Text:='INS';
              False: Panels[4].Text:='';
            end;
            Panels[5].MinWidth:=Canvas.TextWidth('NUM')+10;
            Panels[5].MaxWidth:=Canvas.TextWidth('NUM')+10;
            case ksKeys.Numlock of
              True : Panels[5].Text:='NUM';
              False: Panels[5].Text:='';
            end;
            Panels[6].MinWidth:=Canvas.TextWidth('CAPS')+10;
            Panels[6].MaxWidth:=Canvas.TextWidth('CAPS')+10;
            case ksKeys.Capslock of
              True : Panels[6].Text:='CAPS';
              False: Panels[6].Text:='';
            end;
            Panels[7].MinWidth:=Canvas.TextWidth('SCROLL')+10;
            Panels[7].MaxWidth:=Canvas.TextWidth('SCROLL')+10;
            case ksKeys.Scrolllock of
              True : Panels[7].Text:='SCROLL';
              False: Panels[7].Text:='';
            end;
            for I:=0 to Panels.Count-1 do
              with Panels[I] do
                if not (AutoSize or AutoWidth) then
                  Width:=Canvas.TextWidth(Text)+10;
          end;
      if EditorExist then
        if State=hsReady then
          begin
            if FindDocMenuItem(THFileInf(CurrEdit.Tag))<>nil then
              FindDocMenuItem(THFileInf(CurrEdit.Tag)).Checked:=True;
          end;
    end;
end;


Don't you think thats HUGE? well I think that's huge, and I know, that it doesn't to be in one procedure. I wan't to divide it into few other procedures but I don't know witch part goes to witch procedure.. I could divide it as it's divided by comments, but thats not the way (some of this stuff really shouldnt been taken away from others, others simply got to be used onece or twice in program... can you help me with this?

PostPosted: August 30th, 2003, 2:52 pm
by Kambiz
In my opinion, the part of code related to actions cannot be optimized more.

However, things related to the status bar can be optimized quiet a lot.

Firstly, you clear text of all panels, and then you assign a new or old the previous text to the panels again. In this case, you forced status bar to recalculate size of all panels and redraw them. Don't clear the text before, or at least put entire code inside the BeginUpdate and EndUpdate block.

Secodnly, every time you set the MinWidth and MaxWidth properties of panels. You need to do it just once.

Cheers,
Kambiz

PostPosted: September 2nd, 2003, 9:52 pm
by richardchaven
Are you looking for optimizations like this:
Code: Select all
 if EditorExist then
        TempValue:=CurrEdit.CanUndo
 else
        TempValue:=False;
becomes
Code: Select all
 TempValue := EditorExist and CurrEdit.CanUndo;

??

This sort of optimzation does not affect execution speed and can make the code harder to read; I advise against it.

Cheers

PostPosted: September 2nd, 2003, 9:55 pm
by richardchaven
The only real optimization I can think of that Kambiz did not cover is to store the value of
Code: Select all
Canvas.TextWidth('...')
instead of calling it twice.

Cheers

PostPosted: September 3rd, 2003, 4:21 pm
by Kambiz
richardchaven wrote:The only real optimization I can think of that Kambiz did not cover is to store the value of
Code: Select all
Canvas.TextWidth('...')
instead of calling it twice.


Actually there's no need to store that value. When both AutoSize and AutoWidth properties of the panel are False, Johnny_Bit needs only to set the Width property of the panel.

PostPosted: September 5th, 2003, 10:09 pm
by Johnny_Bit
richardchaven wrote:Are you looking for optimizations like this:
Code: Select all
 if EditorExist then
        TempValue:=CurrEdit.CanUndo
 else
        TempValue:=False;
becomes
Code: Select all
 TempValue := EditorExist and CurrEdit.CanUndo;

??

This sort of optimzation does not affect execution speed and can make the code harder to read; I advise against it.

Cheers



Well hell no. if i call to curredit.canundo when editorexist=false then accesss voliation is raised. This code is simply too big and i don't have idea how to divide it into parts.

PostPosted: September 5th, 2003, 10:34 pm
by Kambiz
As you already mentioned in your first post, deviding to three parts as Updating caption, actions, and status should be fine.

By the way, the following code:

Code: Select all
if EditorExist then
        TempValue:=CurrEdit.CanUndo
 else
        TempValue:=False;

is as same as:

Code: Select all
TempValue := EditorExist and CurrEdit.CanUndo;

In the second form, if the EditorExist variable is false, the rest of the expression is has no effect in the result and Delphi doesn't evaluate it. For more information about short-circuit Boolean expression evaluation please look for BOOLEVAL compiler switch in Delphi help.

richardchaven advised to use the first way (as you did), but personaly I like the second one. :)

PostPosted: September 6th, 2003, 12:51 pm
by Johnny_Bit
Thanks, but i think that my solution is safer... If you can then download program that runs this procedure: HAKGER Text Editor Little and also if You have Win XP then download also HTEL's manifest: HAKGER Text Editor Little Manifest

Just don't laught! this program is far from end of development. tis release is called: "HTEL pre-alpha 0.14.21 ns"

PostPosted: September 6th, 2003, 1:12 pm
by Kambiz
Congratulations!
It is a cute editor.

PostPosted: September 6th, 2003, 5:50 pm
by Johnny_Bit
Thank you. Does it executes without visible lags?

BTW: :oops: this program is in polish... sorry maybe soon i'll make multilanguage support. For now I just hope that glyphs explain more than captions...

BTW2: Under links that I postet before is newest version of program (manifest still is on server)

I've removed useless loop and procedure now looks like that:

Code: Select all
procedure TMain.UpdateInf;
var
  EditorExist, TempValue: Boolean;
  I: Word;
  L: Integer;
  CurrEdit: THMemo;
begin
  CurrEdit:=CurrentEditor;
  if State=hsReady then
    begin
      EditorExist:=(CurrEdit <> nil);
      if EditorExist then
        begin
          with Main do
            begin
              ActiveControl:=CurrentEditor;
              Caption:='HTEL - ['+pcFiles.ActivePage.Caption+']';
              if ClipBoardPreview<>nil then
                ClipBoardPreview.cvClipboardPreview.Font:=CurrEdit.Font;
            end;
          if pcFiles.PageCount=1 then
            pcFiles.HideAllTabs:=True
          else
            begin
              pcFiles.HideAllTabs:=False;
              pcFiles.TabStop:=True;
            end;
        end
      else
        begin
          Main.Caption:=Application.Title;
        end;
//--Actions-Update--------------------------------------------------------------
    //File Actions
      if EditorExist then
        TempValue:=CurrEdit.Modified
      else
        TempValue:=False;
      actFileSave.Enabled:=EditorExist and TempValue;
      actFileSaveAs.Enabled:=EditorExist;
      if EditorExist then
        begin
          for I:=0 to pcFiles.PageCount-1 do
            begin
              TempValue:=TempValue or (pcFiles.Pages[I].Controls[0] as THMemo).Modified
            end;
        end
      else
        TempValue:=True;
      actFileSaveAll.Enabled:=EditorExist and TempValue;
      actFileClose.Enabled:=EditorExist;
      actFileCloseOthers.Enabled:=EditorExist and (pcFiles.PageCount>1);
      actFileCloseAll.Enabled:=EditorExist and (pcFiles.PageCount>1);
      actFileSendMail.Enabled:=EditorExist;
 { TODO 1 -oJohnny_Bit -cActions : Trza jeszcze popracować przy actFilePrint i actFilePrintPreviev }
      actFilePrintPreview.Enabled:=EditorExist and (Printer.Printers.Count>0);
    //Edit Actions
      if EditorExist then
        TempValue:=CurrEdit.CanUndo
      else
        TempValue:=False;
      actEditUndo.Enabled:=EditorExist and TempValue;
      if EditorExist then
        TempValue:=CurrEdit.SelLength>0
      else
        TempValue:=False;
      actEditCut.Enabled:=EditorExist and TempValue;
      actEditCopy.Enabled:=EditorExist and TempValue;
      actEditPaste.Enabled:=EditorExist and (Clipboard.HasFormat(CF_TEXT));
      actEditDelete.Enabled:=EditorExist and TempValue;
      if EditorExist then
        TempValue:=Length(CurrEdit.Text)>0
      else
        TempValue:=False;
      actEditSelectAll.Enabled:=EditorExist and TempValue;
      actEditFont.Enabled:=EditorExist;
      actEditWordWrap.Enabled:=EditorExist;
      if EditorExist then
        TempValue:=CurrEdit.WordWrap
      else
        TempValue:=False;
      actEditWordWrap.Checked:=TempValue;
      actEditUpperCase.Enabled:=EditorExist;
      actEditLowerCase.Enabled:=EditorExist;
      actEditFirstsUpperCase.Enabled:=EditorExist;
      actEditWin1250.Enabled:=EditorExist;
      actEditISO_8859_2.Enabled:=EditorExist;
    //Search Actions
      if EditorExist then
        TempValue:=Length(CurrEdit.Text)>0
      else
        TempValue:=False;
      actSearchFind.Enabled:=EditorExist and TempValue;
      actSearchFindNext.Enabled:=actSearchFind.Enabled and (fdFind.FindText<>'');
      actSearchReplace.Enabled:=EditorExist and TempValue;
    //Insert Actions
      actInsertStdDate.Enabled:=EditorExist;
      actInsertStdTime.Enabled:=EditorExist;
      actInsertStdDateTime.Enabled:=EditorExist;
      actInsertUsrDate.Enabled:=EditorExist;
      actInsertUsrTime.Enabled:=EditorExist;
      actInsertUsrDateTime.Enabled:=EditorExist;
      actInsertInFormat.Enabled:=EditorExist;
      actInsertFromFile.Enabled:=EditorExist;
    //Viev Actions
      sbStatus.Visible:=EditorExist;
      actViewStatusBar.Checked:=sbStatus.Visible;
      actViewStatusBar.Enabled:=EditorExist;
      cbTopBar.Visible:=tlbrToolbar.ButtonCount>0;
    //Help Actions
      {No Help Actions enabel/disable}
//--StatusBar-Update------------------------------------------------------------
      with sbStatus do
        if Visible then //Little optimalization hehe
          begin
            Panels.BeginUpdate;
            if EditorExist then
              Panels[0].ImageIndex:=pcFiles.ActivePage.ImageIndex;
              Panels[1].Text:=Format('Col: %d; Row: %d', [CurrEdit.CaretPos.X, CurrEdit.CaretPos.Y]);
              Panels[1].Width:=Canvas.TextWidth(Panels[1].Text)+10;
              L:=Length(CurrEdit.Text);
              case L of
                0..1024: Panels[2].Text:=Format('%d B', [L]);
                1025..1048576: Panels[2].Text:=Format('%3.2f KB', [L/1024])
                else
                  Panels[2].Text:=Format('%3.2f MB', [L/1048576]);
              end;
            Panels[2].Width:=Canvas.TextWidth(Panels[2].Text)+10;
            Panels[4].Width:=Canvas.TextWidth('INS')+10;
            case ksKeys.Insert of             
              True : Panels[4].Text:='INS';
              False: Panels[4].Text:='';
            end;
            Panels[5].Width:=Canvas.TextWidth('NUM')+10;
            case ksKeys.Numlock of
              True : Panels[5].Text:='NUM';
              False: Panels[5].Text:='';
            end;
            Panels[6].Width:=Canvas.TextWidth('CAPS')+10;
            case ksKeys.Capslock of
              True : Panels[6].Text:='CAPS';
              False: Panels[6].Text:='';
            end;
            Panels[7].Width:=Canvas.TextWidth('SCROLL')+10;
            case ksKeys.Scrolllock of
              True : Panels[7].Text:='SCROLL';
              False: Panels[7].Text:='';
            end;
            Panels.EndUpdate;
          end;
      if EditorExist then
        if State=hsReady then
          begin
            if FindDocMenuItem(THFileInf(CurrEdit.Tag))<>nil then
              FindDocMenuItem(THFileInf(CurrEdit.Tag)).Checked:=True;
          end;
    end;
end;

PostPosted: September 6th, 2003, 6:27 pm
by Kambiz
Actually using glyphs I could manage to use the editor. :)

Why you didn't add the manifest as a resource to the exe?

PostPosted: September 6th, 2003, 6:51 pm
by Johnny_Bit
Kambiz wrote:Why you didn't add the manifest as a resource to the exe?


I don't know. maybe this way is better. well if you think that adding manifest to resource is good idea, then i will do so

One simple question: In scale 1-10 when 5 = notepad how there fits HTEL?

PostPosted: September 6th, 2003, 8:29 pm
by Kambiz
It depends. For example if somebody uses a language, which is available only as unicode cannot use HTEL. But generaly it offers much more options and felexibility than Notebook. In my point of view its score is 8.

By adding the manifest to the executable you do not need to deliver one additional file.

I noticed the toolbar is not visible on my copy. I found "View Toolbar" on the menu, but it doesn't have any effect.

PostPosted: September 7th, 2003, 12:47 pm
by Johnny_Bit
This is funny thing about toolbar. in Menu Widok press last menuitem. Toolbar Configuration dialog should apear, and after configuring (In left Listview there are avilable items, in right are actual items) press OK. now Toolbar should Apear. or if you want I can send you HTEL Toolbar File (*.htf)

Unicode will be soon (when I understand how to detect unicode files)

PostPosted: September 7th, 2003, 3:18 pm
by Kambiz
To manage unicode, in the following links you can find interesting stuffs:
http://www.delphi-unicode.net (currently is down) :(
http://home.ccci.org/wolbrink/tnt/delphi_unicode_controls.htm

And, here is what I see as toolbar: