View Issue Details

IDProjectCategoryView StatusLast Update
0037383PatchesWidgetsetpublic2020-07-18 23:43
ReporterCudaText man Assigned To 
PrioritynormalSeverityminorReproducibilityN/A
Status newResolutionopen 
Summary0037383: gtk2: Simpler storage of pressed keys: FPList->array
DescriptionThis removes all mem allocs on key press/depress.
TagsNo tags attached.
Fixed in Revision
LazTarget
WidgetsetGTK 2
Attached Files

Relationships

related to 0034485 new Lazarus GTK2 does not have getKeyshiftstate() for FormDropFiles 

Activities

CudaText man

2020-07-18 02:41

reporter  

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
k.diff (8,883 bytes)   

CudaText man

2020-07-18 21:19

reporter   ~0124153

Last edited: 2020-07-18 21:20

View 2 revisions

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
k2.diff (8,600 bytes)   

Juha Manninen

2020-07-18 23:42

developer   ~0124158

Last edited: 2020-07-18 23:43

View 2 revisions

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?

Issue History

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