Merge pull request #433 from guiv42/perfOptim3

Perf optimization, update SWT for Linux to 4.26

INSTALL.md

@@ -37,11 +37,11 @@ In order for Asian characters to be displayed correctly, you may also need to in
 ### Download and install SWT for Linux
 
 ```sh
-$ wget https://archive.eclipse.org/eclipse/downloads/drops4/R-4.21-202109060500/swt-4.21-gtk-linux-`uname -m`.zip
-$ mkdir swt-4.21-gtk-linux-`uname -m`
-$ cd swt-4.21-gtk-linux-`uname -m`
-$ unzip ../swt-4.21-gtk-linux-`uname -m`.zip
-$ mvn install:install-file -Dfile=swt.jar -DgroupId=org.eclipse.swt -DartifactId=org.eclipse.swt.gtk.linux -Dpackaging=jar -Dversion=4.21
+$ wget https://archive.eclipse.org/eclipse/downloads/drops4/R-4.26-202211231800/swt-4.26-gtk-linux-`uname -m`.zip
+$ mkdir swt-4.26-gtk-linux-`uname -m`
+$ cd swt-4.26-gtk-linux-`uname -m`
+$ unzip ../swt-4.26-gtk-linux-`uname -m`.zip
+$ mvn install:install-file -Dfile=swt.jar -DgroupId=org.eclipse.swt -DartifactId=org.eclipse.swt.gtk.linux -Dpackaging=jar -Dversion=4.26
 $ cd ..
 ```
 

desktop/TuxGuitar-test/pom.xml

@@ -72,7 +72,7 @@
 		<dependency>
 			<groupId>${org.eclipse.swt.groupId}</groupId>
 			<artifactId>org.eclipse.swt.gtk.linux</artifactId>
-			<version>4.21</version>
+			<version>4.26</version>
 		</dependency>
 	</dependencies>
 </project>

desktop/TuxGuitar-ui-toolkit/src/org/herac/tuxguitar/ui/widget/UIScrollBar.java

@@ -31,4 +31,6 @@ public interface UIScrollBar extends UIComponent {
 	void addSelectionListener(UISelectionListener listener);
 
 	void removeSelectionListener(UISelectionListener listener);
+
+	void setVisible(boolean visible);
 }

desktop/TuxGuitar/src/org/herac/tuxguitar/app/view/component/tab/TGControl.java

@@ -40,19 +40,19 @@ public class TGControl {
 
 	private int scrollX;
 	private int scrollY;
-	private boolean resetScroll;
 	protected long lastVScrollTime;
 	protected long lastHScrollTime;
 
 	private boolean painting;
+	private boolean wasPlaying;
 
 	public TGControl(TGContext context, UIContainer parent) {
 		this.context = context;
 		this.tablature = TablatureEditor.getInstance(this.context).getTablature();
 		this.initialize(parent);
 	}
 
-	public void initialize(UIContainer parent) {
+	private void initialize(UIContainer parent) {
 		UIFactory factory = TGApplication.getInstance(this.context).getFactory();
 		UITableLayout layout = new UITableLayout(0f);
 
@@ -103,85 +103,116 @@ public void onDispose(UIDisposeEvent event) {
 	}
 
 	public void paintTablature(UIPainter painter) {
-		this.setPainting(true);
+		boolean isPlaying;
+		TGMeasureImpl playedMeasure = null;
+
+		this.painting = true;
 		try{
-			this.checkScroll();
-
+			isPlaying = MidiPlayer.getInstance(this.context).isRunning();
 			int oldWidth = this.width;
 			int oldHeight = this.height;
 
-			UIRectangle area = createRectangle(this.canvas.getBounds());
-
-			this.scrollX = this.hScroll.getValue();
-			this.scrollY = this.vScroll.getValue();
+			// determine position in tab (which part shall be displayed): scroll x, y
+			if (isPlaying) {
+				playedMeasure = TGTransport.getInstance(this.context).getCache().getPlayMeasure();
+				if(playedMeasure != null && playedMeasure.hasTrack(this.tablature.getCaret().getTrack().getNumber())){
+					moveTo(playedMeasure);
+				}
+			} else {
+				// if (wasPlaying): keep position in tab unchanged
+				// new scrollbar attributes shall be defined from position in tab, not the opposite
+				// else :
+				if (!wasPlaying) {
+					// follow caret movement or user actions on scrollbars
+					if(this.tablature.getCaret().hasChanges()){
+						this.tablature.getCaret().setChanges(false);
+						this.moveTo(this.tablature.getCaret().getMeasure());
+					} else {
+						this.scrollX = this.hScroll.getValue();
+						this.scrollY = this.vScroll.getValue();
+					}
+				}
+			}
 
-			this.tablature.paintTablature(painter, area, -this.scrollX, -this.scrollY);
+			this.tablature.paintTablature(painter, this.canvas.getBounds(), -this.scrollX, -this.scrollY);
 
 			this.width = Math.round(this.tablature.getViewLayout().getWidth());
 			this.height = Math.round(this.tablature.getViewLayout().getHeight());
 
-			this.updateScroll();
+			// highlight played beat
+			if ( (playedMeasure != null) && playedMeasure.hasTrack(this.tablature.getCaret().getTrack().getNumber())
+					&& !playedMeasure.isOutOfBounds() ){
+				TGBeatImpl playedBeat = TGTransport.getInstance(this.context).getCache().getPlayBeat();
+				this.tablature.getViewLayout().paintPlayMode(painter, playedMeasure, playedBeat);
+			}
 
-			if( MidiPlayer.getInstance(this.context).isRunning()){
-				this.paintTablaturePlayMode(painter);
+			// update scrollbars
+			if (isPlaying) {
+				this.hScroll.setVisible(false);
+				this.vScroll.setVisible(false);
+			} else {
+				this.updateScrollBars();
+				if (wasPlaying) {
+					// player just stopped, redefine scrollbars positions considering position where player stopped
+					this.hScroll.setValue(this.scrollX);
+					this.vScroll.setValue(this.scrollY);
+				}
 			}
-			// Si no estoy reproduciendo y hay cambios
-			// muevo el scroll al compas que tiene el caret
-			else if(this.tablature.getCaret().hasChanges() || (this.width != oldWidth || this.height != oldHeight)){
-				// Mover el scroll puede necesitar redibujar
-				// por eso es importante desmarcar los cambios antes de hacer el moveScrollTo
-				this.tablature.getCaret().setChanges(false);
-
-				this.moveScrollTo(this.tablature.getCaret().getMeasure(), area);
+
+			// tab resized?
+			if ((this.width != oldWidth) || (this.height != oldHeight)) {
+				redraw();
 			}
+
+			this.wasPlaying = isPlaying;
+
 		}catch(Throwable throwable){
 			throwable.printStackTrace();
 		}
-		this.setPainting(false);
+		this.painting = false;
 	}
 
-	private void paintTablaturePlayMode(UIPainter painter){
-		try{
-			TGMeasureImpl measure = TGTransport.getInstance(this.context).getCache().getPlayMeasure();
-			TGBeatImpl beat = TGTransport.getInstance(this.context).getCache().getPlayBeat();
-			if(measure != null && measure.hasTrack(this.tablature.getCaret().getTrack().getNumber())){
-				this.moveScrollTo(measure);
-
-				if(!measure.isOutOfBounds()){
-					this.tablature.getViewLayout().paintPlayMode(painter, measure, beat);
-				}
+	/* Warning: only update scrollbars if at least one attribute has changed
+	 * else it creates a significant performance issue in Linux/SWT configuration:
+	 * - updating scrollbar generates a SWT event to repaint this.canvas, because scrollbars are transparent
+	 * - it calls this.paintTablature
+	 * - which in turn call this.updateScroll
+	 * if scrollbars are updated here, it creates a recursive loop: paintTablature -> updateScroll -> paintTablature -> ...
+	 * this leads to repainting the tab about 60 times per second, creating a significant CPU load
+	 * see https://github.com/helge17/tuxguitar/issues/403
+	 */
+	private void updateScrollBars(){
+		UIRectangle bounds = this.canvas.getBounds();
+
+		int hMax = Math.max(Math.round(this.width - bounds.getWidth()), 0);
+		int hThumb = Math.round(bounds.getWidth());
+		if (hMax>0) {
+			this.hScroll.setVisible(true);
+			if (this.hScroll.getMaximum() != hMax) {
+				this.hScroll.setMaximum(hMax);
 			}
-		}catch(Throwable throwable){
-			throwable.printStackTrace();
+			if (this.hScroll.getThumb() != hThumb) {
+				this.hScroll.setThumb(hThumb);
+			}
+		} else {
+			this.hScroll.setVisible(false);
 		}
-	}
-
-	public void resetScroll(){
-		this.resetScroll = true;
-	}
-
-	public void checkScroll(){
-		if( this.resetScroll ){
-			this.hScroll.setValue(0);
-			this.vScroll.setValue(0);
-			this.resetScroll = false;
+		int vMax = Math.max(Math.round(this.height - bounds.getHeight()), 0);
+		int vThumb = Math.round(bounds.getHeight());
+		if (vMax>0) {
+			this.vScroll.setVisible(true);
+			if (this.vScroll.getMaximum() != vMax) {
+				this.vScroll.setMaximum(vMax);
+			}
+			if (this.vScroll.getThumb() != vThumb) {
+				this.vScroll.setThumb(vThumb);
+			}
+		} else {
+			this.vScroll.setVisible(false);
 		}
 	}
-
-	public void updateScroll(){
-		UIRectangle bounds = this.canvas.getBounds();
-
-		this.hScroll.setMaximum(Math.max(Math.round(this.width - bounds.getWidth()), 0));
-		this.vScroll.setMaximum(Math.max(Math.round(this.height - bounds.getHeight()), 0));
-		this.hScroll.setThumb(Math.round(bounds.getWidth()));
-		this.vScroll.setThumb(Math.round(bounds.getHeight()));
-	}
-
-	public void moveScrollTo(TGMeasureImpl measure){
-		this.moveScrollTo(measure, createRectangle(this.canvas.getBounds()));
-	}
-
-	public void moveScrollTo(TGMeasureImpl measure, UIRectangle area) {
+
+	private void moveTo(TGMeasureImpl measure) {
 		if( measure != null && measure.getTs() != null ){
 			int mX = Math.round(measure.getPosX());
 			int mY = Math.round(measure.getPosY());
@@ -191,26 +222,18 @@ public void moveScrollTo(TGMeasureImpl measure, UIRectangle area) {
 			int marginHeight = Math.round(this.tablature.getViewLayout().getFirstTrackSpacing());
 			boolean playMode = MidiPlayer.getInstance(this.context).isRunning();
 
-			//Solo se ajusta si es necesario
-			Integer hScrollValue = this.computeScrollValue(this.scrollX, mX, mWidth, marginWidth, Math.round(area.getWidth()), this.width, playMode);
+			Integer hScrollValue = this.computeScrollValue(this.scrollX, mX, mWidth, marginWidth, Math.round(this.canvas.getBounds().getWidth()), this.width, playMode);
 			if( hScrollValue != null ) {
-				this.hScroll.setValue(hScrollValue);
+				this.scrollX = hScrollValue;
 			}
-
-			//Solo se ajusta si es necesario
-			Integer vScrollValue = this.computeScrollValue(this.scrollY, mY, mHeight, marginHeight, Math.round(area.getHeight()), this.height, playMode);
+			Integer vScrollValue = this.computeScrollValue(this.scrollY, mY, mHeight, marginHeight, Math.round(this.canvas.getBounds().getHeight()), this.height, playMode);
 			if( vScrollValue != null ) {
-				this.vScroll.setValue(vScrollValue);
-			}
-
-			// Si cambio el valor de algun scroll redibuja la pantalla
-			if( this.scrollX != this.hScroll.getValue() || this.scrollY != this.vScroll.getValue() ){
-				redraw();
+				this.scrollY = vScrollValue;
 			}
 		}
 	}
 
-	public Integer computeScrollValue(int scrollPos, int mPos, int mSize, int mMargin, int areaSize, int fullSize, boolean playMode) {
+	private Integer computeScrollValue(int scrollPos, int mPos, int mSize, int mMargin, int areaSize, int fullSize, boolean playMode) {
 		Integer value = null;
 
 		// when position is less than scroll
@@ -237,37 +260,17 @@ public void setFocus() {
 
 	public void redraw(){
 		if(!this.isDisposed() ){
-			this.setPainting(true);
+			this.painting = true;
 			this.canvas.redraw();
 		}
 	}
 
 	public void redrawPlayingMode() {
-		if(!this.isDisposed() && !this.isPainting() && MidiPlayer.getInstance(this.context).isRunning()) {
+		if(!this.isDisposed() && !this.painting && MidiPlayer.getInstance(this.context).isRunning()) {
 			this.redraw();
 		}
 	}
 
-	public boolean isPainting() {
-		return this.painting;
-	}
-
-	public void setPainting(boolean painting) {
-		this.painting = painting;
-	}
-
-	public TGContext getContext() {
-		return this.context;
-	}
-
-	public Tablature getTablature() {
-		return tablature;
-	}
-
-	public UIContainer getContainer() {
-		return container;
-	}
-
 	public UICanvas getCanvas() {
 		return canvas;
 	}
@@ -276,7 +279,4 @@ public boolean isDisposed() {
 		return (this.container == null || this.container.isDisposed() || this.canvas == null || this.canvas.isDisposed());
 	}
 
-	public UIRectangle createRectangle(UIRectangle rectangle){
-		return new UIRectangle(rectangle.getX(),rectangle.getY(),rectangle.getWidth(),rectangle.getHeight());
-	}
 }

desktop/pom.xml

@@ -295,7 +295,7 @@
 			<dependency>
 				<groupId>${org.eclipse.swt.groupId}</groupId>
 				<artifactId>org.eclipse.swt.gtk.linux</artifactId>
-				<version>4.21</version>
+				<version>4.26</version>
 			</dependency>
 			<dependency>
 				<groupId>${org.eclipse.swt.groupId}</groupId>

doc/contribute.md

@@ -146,11 +146,11 @@ To have all foreign packages in your Eclipse workspace, create a folder named "e
 eclipse-workspace$ sudo apt install maven
 eclipse-workspace$ mkdir externals
 eclipse-workspace$ cd externals
-eclipse-workspace$ wget https://archive.eclipse.org/eclipse/downloads/drops4/R-4.21-202109060500/swt-4.21-gtk-linux-x86_64.zip
-eclipse-workspace/externals$ mkdir swt-4.21-gtk-linux-x86_64
-eclipse-workspace/externals$ cd swt-4.21-gtk-linux-x86_64
-eclipse-workspace/externals$ unzip ../swt-4.21-gtk-linux-x86_64.zip
-eclipse-workspace/externals$ mvn install:install-file -Dfile=swt.jar -DgroupId=org.eclipse.swt -DartifactId=org.eclipse.swt.gtk.linux -Dpackaging=jar -Dversion=4.21
+eclipse-workspace$ wget https://archive.eclipse.org/eclipse/downloads/drops4/R-4.26-202211231800/swt-4.26-gtk-linux-x86_64.zip
+eclipse-workspace/externals$ mkdir swt-4.26-gtk-linux-x86_64
+eclipse-workspace/externals$ cd swt-4.26-gtk-linux-x86_64
+eclipse-workspace/externals$ unzip ../swt-4.26-gtk-linux-x86_64.zip
+eclipse-workspace/externals$ mvn install:install-file -Dfile=swt.jar -DgroupId=org.eclipse.swt -DartifactId=org.eclipse.swt.gtk.linux -Dpackaging=jar -Dversion=4.26
 ```
 
 #### Define run configuration

misc/build_tuxguitar_from_source.sh

@@ -636,8 +636,8 @@ fi
 # Linux
 if [ $build_linux ]; then
   # SWT version in Debian 12
-  SWT_VERSION=4.21
-  SWT_DATE=202109060500
+  SWT_VERSION=4.26
+  SWT_DATE=202211231800
   SWT_PLATFORM=gtk-linux
   [ `uname` == Linux ] && build_tg_for_linux
 fi