View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0037383 | Patches | Widgetset | public | 2020-07-18 02:41 | 2021-02-22 15:38 |
Reporter | CudaText man | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | N/A |
Status | new | Resolution | open | ||
Summary | 0037383: gtk2: Simpler storage of pressed keys: FPList->array | ||||
Description | This removes all mem allocs on key press/depress. | ||||
Tags | No tags attached. | ||||
Fixed in Revision | |||||
LazTarget | |||||
Widgetset | GTK 2 | ||||
Attached Files |
|
related to | 0034485 | new | Lazarus | GTK2 does not have getKeyshiftstate() for FormDropFiles |
|
k.diff (8,883 bytes)
Index: lcl/interfaces/gtk2/gtk2callback.inc =================================================================== --- lcl/interfaces/gtk2/gtk2callback.inc (revision 63594) +++ lcl/interfaces/gtk2/gtk2callback.inc (working copy) @@ -111,6 +111,70 @@ Result := DeliverPaintMessage(Target, Msg); end; +{ TPressedKeysRecord } + +procedure TPressedKeysRecord.Clear; +begin + FillChar(Ar, SizeOf(Ar), 0); +end; + +function TPressedKeysRecord.Has(V: integer): Boolean; +var + i: integer; +begin + if V = 0 then + exit(False); + for i := 0 to High(Ar) do + if Ar[i] = V then + exit(True); + Result := False; +end; + +procedure TPressedKeysRecord.Add(V: integer); +var + i: integer; +begin + for i := 0 to High(Ar) do + if Ar[i] = 0 then + begin + Ar[i] := V; + Exit; + end; +end; + +procedure TPressedKeysRecord.Remove(V: integer); +var + i: integer; +begin + for i := 0 to High(Ar) do + if Ar[i] = V then + begin + Ar[i] := 0; + //Exit; //better remove all occurrences + end; +end; + +procedure TPressedKeysRecord.Update(V: integer; DoAdd: boolean); +begin + if V = 0 then Exit; + if DoAdd then + begin + if not Has(V) then + Add(V); + end + else + Remove(V); +end; + +procedure TPressedKeysRecord.Toggle(V: integer); +begin + if Has(V) then + Remove(V) + else + Add(V); +end; + + procedure EventTrace(const TheMessage : string; data : pointer); begin // if Data = nil then @@ -786,19 +850,11 @@ var NeedShiftUpdateAfterFocus: Boolean; -procedure UpdateShiftState(const KeyStateList: TFPList; const ShiftState: TShiftState); +procedure UpdateShiftState(const ShiftState: TShiftState); - procedure UpdateList(const AVKeyCode: Integer; const APressed: Boolean); + procedure UpdateList(ACode: Integer; APressed: Boolean); inline; begin - if AVKeyCode = 0 then Exit; - if APressed - then begin - if KeyStateList.IndexOf({%H-}Pointer(PtrUInt(AVKeyCode))) < 0 - then KeyStateList.Add({%H-}Pointer(PtrUInt(AVKeyCode))); - end - else begin - KeyStateList.Remove({%H-}Pointer(PtrUInt(AVKeyCode))); - end; + PressedKeys.Update(ACode, APressed); end; const @@ -857,7 +913,7 @@ NeedShiftUpdateAfterFocus := False; gdk_window_get_pointer(nil, nil, nil, @Mask); - UpdateShiftState(GTK2WidgetSet.KeyStateList, GTKEventStateToShiftState(Word(Mask))); + UpdateShiftState(GTKEventStateToShiftState(Word(Mask))); {$IFDEF VerboseFocus} DebugLnEnter(['GTKFocusCB INIT Widget=',DbgS(Widget),' Event^.theIn=',Event^._in, @@ -1457,7 +1513,7 @@ NeedShiftUpdateAfterFocus := False; LastModifierKeys := ShiftState*[ssShift, ssCtrl, ssAlt, ssSuper]; //DebugLn(['Adjust KeyStateList in MouseMove',Integer(LastModifierKeys)]); - UpdateShiftState(GTK2WidgetSet.KeyStateList, LastModifierKeys); + UpdateShiftState(LastModifierKeys); end; {$IFDEF VerboseMouseCapture} @@ -1979,7 +2035,7 @@ then begin LastModifierKeys := ShiftState*[ssShift, ssCtrl, ssAlt, ssSuper]; //DebugLn(['Adjust KeyStateList in MouseBtnDown',Integer(LastModifierKeys)]); - UpdateShiftState(GTK2WidgetSet.KeyStateList, LastModifierKeys); + UpdateShiftState(LastModifierKeys); end; MappedXY := TranslateGdkPointToClientArea(Event^.Window, EventXY, @@ -3189,29 +3245,18 @@ ------------------------------------------------------------------------------} function GTKKeySnooper(Widget: PGtkWidget; Event: PGdkEventKey; FuncData: gPointer): gInt; cdecl; -var - KeyStateList: TFPList absolute FuncData; - procedure UpdateToggleList(const AVKeyCode: Integer); + procedure UpdateToggleList(ACode: Integer); inline; begin // Check for a toggle // If the remove was successfull, the key was on // else it was off so we should set the toggle flag - if KeyStateList.Remove({%H-}Pointer(PtrUInt(AVKeyCode or KEYMAP_TOGGLE))) < 0 - then KeyStateList.Add({%H-}Pointer(PtrUInt(AVKeyCode or KEYMAP_TOGGLE))); + PressedKeys.Toggle(ACode or KEYMAP_TOGGLE); end; - procedure UpdateList(const AVKeyCode: Integer; const APressed: Boolean); + procedure UpdateList(ACode: Integer; APressed: Boolean); inline; begin - if AVKeyCode = 0 then Exit; - if APressed - then begin - if KeyStateList.IndexOf({%H-}Pointer(PtrUInt(AVKeyCode))) < 0 - then KeyStateList.Add({%H-}Pointer(PtrUInt(AVKeyCode))); - end - else begin - KeyStateList.Remove({%H-}Pointer(PtrUInt(AVKeyCode))); - end; + PressedKeys.Update(ACode, APressed); end; const @@ -3275,8 +3320,6 @@ Exit; end; - if KeyStateList = nil then exit; - ShiftState := GTKEventStateToShiftState(Event^.State); if (KCInfo.Flags and KCINFO_FLAG_SHIFT_XOR_NUM <> 0) Index: lcl/interfaces/gtk2/gtk2int.pas =================================================================== --- lcl/interfaces/gtk2/gtk2int.pas (revision 63594) +++ lcl/interfaces/gtk2/gtk2int.pas (working copy) @@ -94,7 +94,6 @@ protected function CreateThemeServices: TThemeServices; override; protected - FKeyStateList_: TFPList; // Keeps track of which keys are pressed FDeviceContexts: TDynHashArray;// hasharray of HDC FGDIObjects: TDynHashArray; // hasharray of PGdiObject FMessageQueue: TGtkMessageQueue; // queue of PMsg (must be thread safe!) @@ -327,7 +326,6 @@ property LastFocusIn: PGtkWidget read FLastFocusIn write FLastFocusIn; property LastFocusOut: PGtkWidget read FLastFocusOut write FLastFocusOut; property MultiThreadingEnabled: boolean read FMultiThreadingEnabled; - property KeyStateList: TFPList read FKeyStateList_; end; {$I gtk2listslh.inc} Index: lcl/interfaces/gtk2/gtk2proc.pp =================================================================== --- lcl/interfaces/gtk2/gtk2proc.pp (revision 63594) +++ lcl/interfaces/gtk2/gtk2proc.pp (working copy) @@ -16,6 +16,7 @@ unit Gtk2Proc; {$mode objfpc}{$H+} +{$ModeSwitch advancedrecords} interface @@ -84,7 +85,26 @@ {$endif} +type + + { TPressedKeysRecord } + + TPressedKeysRecord = record + private + Ar: array[0..20] of integer; + public + procedure Clear; + function Has(V: integer): Boolean; + procedure Add(V: integer); + procedure Remove(V: integer); + procedure Update(V: integer; DoAdd: boolean); + procedure Toggle(V: integer); + end; + var + PressedKeys: TPressedKeysRecord; + +var GTKAPIWidget_Type: GType = 0; // GTKCallback.inc headers @@ -985,6 +1005,7 @@ {$I gtk2callback.inc} initialization + PressedKeys.Clear; InitGTKProc; finalization Index: lcl/interfaces/gtk2/gtk2widgetset.inc =================================================================== --- lcl/interfaces/gtk2/gtk2widgetset.inc (revision 63594) +++ lcl/interfaces/gtk2/gtk2widgetset.inc (working copy) @@ -1428,9 +1428,6 @@ FFixWidgetsResized := TDynHashArray.Create(-1); FTimerData := TFPList.Create; - {$IFDEF Use_KeyStateList} - FKeyStateList_ := TFPList.Create; - {$ENDIF} DestroyConnectedWidgetCB:=@DestroyConnectedWidget; @@ -1452,11 +1449,7 @@ // Initialize Stringlist for holding styles Styles := TStringlist.Create; - {$IFDEF Use_KeyStateList} - gtk_key_snooper_install(@GTKKeySnooper, FKeyStateList_); - {$ELSE} gtk_key_snooper_install(@GTKKeySnooper, nil); - {$ENDIF} // Init tooltips FGTKToolTips := gtk_tooltips_new; @@ -1744,9 +1737,6 @@ FreeAndNil(FMessageQueue); FreeAndNil(FDeviceContexts); FreeAndNil(FGDIObjects); - {$IFDEF Use_KeyStateList} - FreeAndNil(FKeyStateList_); - {$ENDIF} FreeAndNil(FTimerData); GtkDefDone; Index: lcl/interfaces/gtk2/gtk2winapi.inc =================================================================== --- lcl/interfaces/gtk2/gtk2winapi.inc (revision 63594) +++ lcl/interfaces/gtk2/gtk2winapi.inc (working copy) @@ -4969,26 +4969,17 @@ VK_LMENU: nVirtKey := VK_MENU; end; - {$IFDEF Use_KeyStateList} - Result := KEYSTATE[FKeyStateList_.IndexOf({%H-}Pointer(PtrUInt(nVirtKey))) >=0]; - {$ELSE} - Implement this - {$ENDIF} + Result := KEYSTATE[PressedKeys.Has(nVirtKey)]; // try extended keys if Result = 0 then begin - {$IFDEF Use_KeyStateList} - Result := KEYSTATE[FKeyStateList_.IndexOf({%H-}Pointer(PtrUInt(nVirtKey or KEYMAP_EXTENDED))) >=0]; - {$ELSE} - Implement this - {$ENDIF} + Result := KEYSTATE[PressedKeys.Has(nVirtKey or KEYMAP_EXTENDED)]; end; {$IFDEF Use_KeyStateList} // add toggle - Result := Result or TOGGLESTATE[FKeyStateList_.IndexOf({%H-}Pointer( - PtrUInt(nVirtKey or KEYMAP_TOGGLE))) >=0]; + Result := Result or TOGGLESTATE[PressedKeys.Has(nVirtKey or KEYMAP_TOGGLE)]; // If there are tons of new keyboard errors this is probably the cause GdkModMask := gtk_accelerator_get_default_mod_mask; if (Result and StateDown) <> 0 then |
|
Changed; now it's only refactor. no "array" here, it's not good to write stuff here in core. patch makes the code simpler/ easier to read. k2.diff (8,600 bytes)
Index: lcl/interfaces/gtk2/gtk2callback.inc =================================================================== --- lcl/interfaces/gtk2/gtk2callback.inc (revision 63594) +++ lcl/interfaces/gtk2/gtk2callback.inc (working copy) @@ -786,19 +786,11 @@ var NeedShiftUpdateAfterFocus: Boolean; -procedure UpdateShiftState(const KeyStateList: TFPList; const ShiftState: TShiftState); +procedure UpdateShiftState(const ShiftState: TShiftState); - procedure UpdateList(const AVKeyCode: Integer; const APressed: Boolean); + procedure UpdateList(ACode: Integer; APressed: Boolean); inline; begin - if AVKeyCode = 0 then Exit; - if APressed - then begin - if KeyStateList.IndexOf({%H-}Pointer(PtrUInt(AVKeyCode))) < 0 - then KeyStateList.Add({%H-}Pointer(PtrUInt(AVKeyCode))); - end - else begin - KeyStateList.Remove({%H-}Pointer(PtrUInt(AVKeyCode))); - end; + FPressedKeys.Update(ACode, APressed); end; const @@ -857,7 +849,7 @@ NeedShiftUpdateAfterFocus := False; gdk_window_get_pointer(nil, nil, nil, @Mask); - UpdateShiftState(GTK2WidgetSet.KeyStateList, GTKEventStateToShiftState(Word(Mask))); + UpdateShiftState(GTKEventStateToShiftState(Word(Mask))); {$IFDEF VerboseFocus} DebugLnEnter(['GTKFocusCB INIT Widget=',DbgS(Widget),' Event^.theIn=',Event^._in, @@ -1457,7 +1449,7 @@ NeedShiftUpdateAfterFocus := False; LastModifierKeys := ShiftState*[ssShift, ssCtrl, ssAlt, ssSuper]; //DebugLn(['Adjust KeyStateList in MouseMove',Integer(LastModifierKeys)]); - UpdateShiftState(GTK2WidgetSet.KeyStateList, LastModifierKeys); + UpdateShiftState(LastModifierKeys); end; {$IFDEF VerboseMouseCapture} @@ -1979,7 +1971,7 @@ then begin LastModifierKeys := ShiftState*[ssShift, ssCtrl, ssAlt, ssSuper]; //DebugLn(['Adjust KeyStateList in MouseBtnDown',Integer(LastModifierKeys)]); - UpdateShiftState(GTK2WidgetSet.KeyStateList, LastModifierKeys); + UpdateShiftState(LastModifierKeys); end; MappedXY := TranslateGdkPointToClientArea(Event^.Window, EventXY, @@ -3189,29 +3181,18 @@ ------------------------------------------------------------------------------} function GTKKeySnooper(Widget: PGtkWidget; Event: PGdkEventKey; FuncData: gPointer): gInt; cdecl; -var - KeyStateList: TFPList absolute FuncData; - procedure UpdateToggleList(const AVKeyCode: Integer); + procedure UpdateToggleList(ACode: Integer); inline; begin // Check for a toggle // If the remove was successfull, the key was on // else it was off so we should set the toggle flag - if KeyStateList.Remove({%H-}Pointer(PtrUInt(AVKeyCode or KEYMAP_TOGGLE))) < 0 - then KeyStateList.Add({%H-}Pointer(PtrUInt(AVKeyCode or KEYMAP_TOGGLE))); + FPressedKeys.Toggle(ACode or KEYMAP_TOGGLE); end; - procedure UpdateList(const AVKeyCode: Integer; const APressed: Boolean); + procedure UpdateList(ACode: Integer; APressed: Boolean); inline; begin - if AVKeyCode = 0 then Exit; - if APressed - then begin - if KeyStateList.IndexOf({%H-}Pointer(PtrUInt(AVKeyCode))) < 0 - then KeyStateList.Add({%H-}Pointer(PtrUInt(AVKeyCode))); - end - else begin - KeyStateList.Remove({%H-}Pointer(PtrUInt(AVKeyCode))); - end; + FPressedKeys.Update(ACode, APressed); end; const @@ -3275,8 +3256,8 @@ Exit; end; - if KeyStateList = nil then exit; - + if FuncData = nil then Exit; + ShiftState := GTKEventStateToShiftState(Event^.State); if (KCInfo.Flags and KCINFO_FLAG_SHIFT_XOR_NUM <> 0) Index: lcl/interfaces/gtk2/gtk2globals.pp =================================================================== --- lcl/interfaces/gtk2/gtk2globals.pp (revision 63594) +++ lcl/interfaces/gtk2/gtk2globals.pp (working copy) @@ -200,7 +200,28 @@ // FTimerData contains the currently running timers FTimerData : TFPList; // list of PGtkITimerinfo +type + + { TPressedKeysData } + + TPressedKeysData = class + private + L: TFPList; + public + constructor Create; + destructor Destroy; override; + function Has(V: integer): Boolean; inline; + procedure Add(V: integer); inline; + procedure Remove(V: integer); inline; + procedure Update(V: integer; DoAdd: boolean); + procedure Toggle(V: integer); + end; + var + // Keeps track of which keys are pressed + FPressedKeys: TPressedKeysData; + +var gtk_handler_quark: TGQuark; @@ -528,6 +549,56 @@ inherited Destroy; end; +{ TPressedKeysData } + +constructor TPressedKeysData.Create; +begin + L := TFPList.Create; +end; + +destructor TPressedKeysData.Destroy; +begin + FreeAndNil(L); +end; + +function TPressedKeysData.Has(V: integer): Boolean; +begin + if V = 0 then + exit(False); + Result := L.IndexOf(Pointer(PtrUInt(V))) >= 0; +end; + +procedure TPressedKeysData.Add(V: integer); +begin + L.Add(Pointer(PtrUInt(V))); +end; + +procedure TPressedKeysData.Remove(V: integer); +begin + L.Remove(Pointer(PtrUInt(V))); +end; + +procedure TPressedKeysData.Update(V: integer; DoAdd: boolean); +begin + if V = 0 then Exit; + if DoAdd then + begin + if not Has(V) then + Add(V); + end + else + Remove(V); +end; + +procedure TPressedKeysData.Toggle(V: integer); +begin + if Has(V) then + Remove(V) + else + Add(V); +end; + + initialization InternalInit; Index: lcl/interfaces/gtk2/gtk2int.pas =================================================================== --- lcl/interfaces/gtk2/gtk2int.pas (revision 63594) +++ lcl/interfaces/gtk2/gtk2int.pas (working copy) @@ -94,7 +94,6 @@ protected function CreateThemeServices: TThemeServices; override; protected - FKeyStateList_: TFPList; // Keeps track of which keys are pressed FDeviceContexts: TDynHashArray;// hasharray of HDC FGDIObjects: TDynHashArray; // hasharray of PGdiObject FMessageQueue: TGtkMessageQueue; // queue of PMsg (must be thread safe!) @@ -327,7 +326,6 @@ property LastFocusIn: PGtkWidget read FLastFocusIn write FLastFocusIn; property LastFocusOut: PGtkWidget read FLastFocusOut write FLastFocusOut; property MultiThreadingEnabled: boolean read FMultiThreadingEnabled; - property KeyStateList: TFPList read FKeyStateList_; end; {$I gtk2listslh.inc} Index: lcl/interfaces/gtk2/gtk2widgetset.inc =================================================================== --- lcl/interfaces/gtk2/gtk2widgetset.inc (revision 63594) +++ lcl/interfaces/gtk2/gtk2widgetset.inc (working copy) @@ -1428,8 +1428,9 @@ FFixWidgetsResized := TDynHashArray.Create(-1); FTimerData := TFPList.Create; + {$IFDEF Use_KeyStateList} - FKeyStateList_ := TFPList.Create; + FPressedKeys := TPressedKeysData.Create; {$ENDIF} DestroyConnectedWidgetCB:=@DestroyConnectedWidget; @@ -1453,7 +1454,7 @@ Styles := TStringlist.Create; {$IFDEF Use_KeyStateList} - gtk_key_snooper_install(@GTKKeySnooper, FKeyStateList_); + gtk_key_snooper_install(@GTKKeySnooper, pointer(1)); // with dummy value {$ELSE} gtk_key_snooper_install(@GTKKeySnooper, nil); {$ENDIF} @@ -1745,7 +1746,7 @@ FreeAndNil(FDeviceContexts); FreeAndNil(FGDIObjects); {$IFDEF Use_KeyStateList} - FreeAndNil(FKeyStateList_); + FreeAndNil(FPressedKeys); {$ENDIF} FreeAndNil(FTimerData); Index: lcl/interfaces/gtk2/gtk2winapi.inc =================================================================== --- lcl/interfaces/gtk2/gtk2winapi.inc (revision 63594) +++ lcl/interfaces/gtk2/gtk2winapi.inc (working copy) @@ -4968,9 +4968,9 @@ VK_LCONTROL: nVirtKey := VK_CONTROL; VK_LMENU: nVirtKey := VK_MENU; end; - + {$IFDEF Use_KeyStateList} - Result := KEYSTATE[FKeyStateList_.IndexOf({%H-}Pointer(PtrUInt(nVirtKey))) >=0]; + Result := KEYSTATE[FPressedKeys.Has(nVirtKey)]; {$ELSE} Implement this {$ENDIF} @@ -4979,7 +4979,7 @@ if Result = 0 then begin {$IFDEF Use_KeyStateList} - Result := KEYSTATE[FKeyStateList_.IndexOf({%H-}Pointer(PtrUInt(nVirtKey or KEYMAP_EXTENDED))) >=0]; + Result := KEYSTATE[FPressedKeys.Has(nVirtKey or KEYMAP_EXTENDED)]; {$ELSE} Implement this {$ENDIF} @@ -4987,8 +4987,7 @@ {$IFDEF Use_KeyStateList} // add toggle - Result := Result or TOGGLESTATE[FKeyStateList_.IndexOf({%H-}Pointer( - PtrUInt(nVirtKey or KEYMAP_TOGGLE))) >=0]; + Result := Result or TOGGLESTATE[FPressedKeys.Has(nVirtKey or KEYMAP_TOGGLE)]; // If there are tons of new keyboard errors this is probably the cause GdkModMask := gtk_accelerator_get_default_mod_mask; if (Result and StateDown) <> 0 then |
|
As a pure optimization this does not make a big difference. Did you measure how much time the allocations take in a real application? However the code touches getKeyshiftstate() which is not implemented. See related issue. Do you know how to implement it? Would it be easier after the patch? |
|
I missed notification, sorry. I didnt measure the time, but it's good small optimization. >getKeyshiftstate() which is not implemented I don't know how to do. |
|
Not (yet) looked at details.... But does that help with the crash in ~c122862 ? |
|
I didn't fix anything, any crash, I just optimized. |
Date Modified | Username | Field | Change |
---|---|---|---|
2020-07-18 02:41 | CudaText man | New Issue | |
2020-07-18 02:41 | CudaText man | File Added: k.diff | |
2020-07-18 16:53 | Juha Manninen | Relationship added | related to 0034485 |
2020-07-18 21:19 | CudaText man | Note Added: 0124153 | |
2020-07-18 21:19 | CudaText man | File Added: k2.diff | |
2020-07-18 21:20 | CudaText man | Note Edited: 0124153 | View Revisions |
2020-07-18 23:42 | Juha Manninen | Note Added: 0124158 | |
2020-07-18 23:43 | Juha Manninen | Note Edited: 0124158 | View Revisions |
2021-02-22 13:33 | CudaText man | Note Added: 0129098 | |
2021-02-22 14:20 | Martin Friebe | Note Added: 0129100 | |
2021-02-22 15:38 | CudaText man | Note Added: 0129101 |