Jump to content

Photo

UI code refactoring and bug fixes


15 replies to this topic

#1 blind_io OFFLINE  

blind_io

    Combat Commando

  • 8 posts
  • Location:Stockholm, Sweden

Posted Thu May 5, 2016 5:10 AM

I accidentally fixed some problems I had with RespeQt. Before I tell you what, let me tell you the background story.

 
A month ago or so I bought a SIO2USB from Lotharek but I couldn't get it to work on my Mac. At first I tried with AspeQt and after some googling I found out about RespeQt, but neither would work. To rule out broken hardware, I also dug out my old PC and it worked without a problem. Since I am a programmer by trade I downloaded QtCreator and forked the RespeQt code on github. I thought there was some weird issue in the serial port setup that caused my problems.
 
I identified the problem to the DSR signal. It never changed.
 
No matter what I did I couldn't get it to work. I was really close to posting here and ask for help when it struck me; I should install the driver from FTDI instead of the default from apple. Lo and behold, it worked!
 
While I was trying to find the issue, I found some cases of peculiar code and some crash bugs.
 
On a whim a few days later I started looking at the UI code. Perhaps I shouldn't have, becuse it trigger me to start "fixing" stuff that I didn't like. It ended up with a binary approximately 20% smaller but with the same functionality.
 
All my changes are pushed to my fork on github. I would make a pull request, but since these changes comes from out of the blue, I thought you might want to look at them first.
 
Since I've only tested the changes on my Mac (OSX 10.11.4) and the SIO2USB, it would be great if someone could test it with other machines and hardware.
 
Check the blind-fixes branch on https://github.com/blind/RespeQt/for all my changes.
 
PS. This is my first post on AtariAge. Hello everyone! 


#2 TheMontezuma OFFLINE  

TheMontezuma

    Dragonstomper

  • 676 posts
  • Location:Hildesheim, D / Kraków, PL

Posted Sat May 7, 2016 2:15 AM

Hello blind_io,

It is great to have a GUI expert joining the community.

I have just created a local branch to test your fixes.

It seems that you use C++11 features, so I needed to add the following setting to compile it:

-std=c++11

I wondered if you changed the layout of the main window intensionally?

Regards

The.Montezuma



#3 blind_io OFFLINE  

blind_io

    Combat Commando

  • Topic Starter
  • 8 posts
  • Location:Stockholm, Sweden

Posted Sat May 7, 2016 3:21 AM

I wouldn't call myself a GUI expert, but I did a project at work using Qt a few years back.

 

Requiring C++11 was not intentional, it was just me being lazy. I just pushed a fix for that to github. Hopefully it should now compile without C++11.

 

The only thing different in the main window should be the offset of the widgets in the right column (disks 9 to O). It is a side effect of adding the widgets from code. Since I'm not a Qt professional, I couldn't find a easy way of fixing the alignment. But my intention is to fix them so they align again.

Also I seems to have committed the native placement of the menu bar that have been discussed on github. As a mac user, anything else just feels wrong. I don't really know how that mode affects other systems, and i don't know what is preferred by other users.

 

If there are other changes to the layout in the main window, please screenshot it so I can see how it looks for you.

 

 



#4 DrVenkman ONLINE  

DrVenkman

    River Patroller

  • 3,983 posts
  • Back off, man! I'm a scientist.
  • Location:KMBT

Posted Sat May 7, 2016 9:15 AM

 

I accidentally fixed some problems I had with RespeQt. Before I tell you what, let me tell you the background story.

 
A month ago or so I bought a SIO2USB from Lotharek but I couldn't get it to work on my Mac. At first I tried with AspeQt and after some googling I found out about RespeQt, but neither would work. To rule out broken hardware, I also dug out my old PC and it worked without a problem. Since I am a programmer by trade I downloaded QtCreator and forked the RespeQt code on github. I thought there was some weird issue in the serial port setup that caused my problems.
 
I identified the problem to the DSR signal. It never changed.
 
No matter what I did I couldn't get it to work. I was really close to posting here and ask for help when it struck me; I should install the driver from FTDI instead of the default from apple. Lo and behold, it worked!
 
While I was trying to find the issue, I found some cases of peculiar code and some crash bugs.
 
On a whim a few days later I started looking at the UI code. Perhaps I shouldn't have, becuse it trigger me to start "fixing" stuff that I didn't like. It ended up with a binary approximately 20% smaller but with the same functionality.
 
All my changes are pushed to my fork on github. I would make a pull request, but since these changes comes from out of the blue, I thought you might want to look at them first.
 
Since I've only tested the changes on my Mac (OSX 10.11.4) and the SIO2USB, it would be great if someone could test it with other machines and hardware.
 
Check the blind-fixes branch on https://github.com/blind/RespeQt/for all my changes.
 
PS. This is my first post on AtariAge. Hello everyone! 

 

 

 

Well,  I just downloaded the latest source and built the executable for 10.11.5 (latest beta) and Xcode 7.3.1. After deployment and copying all the $folders into the app bundle (still haven't figured out how to do that automagically) I get an executable just as big as before. *shrug*



#5 blind_io OFFLINE  

blind_io

    Combat Commando

  • Topic Starter
  • 8 posts
  • Location:Stockholm, Sweden

Posted Sat May 7, 2016 3:18 PM

 

 

Well,  I just downloaded the latest source and built the executable for 10.11.5 (latest beta) and Xcode 7.3.1. After deployment and copying all the $folders into the app bundle (still haven't figured out how to do that automagically) I get an executable just as big as before. *shrug*

 

 

Strange, I just did clean builds of both the master branch and my blind-fixes branch and got a size decrease (of the main binary, not including frameworks and other bundled files) from 1753084 bytes to 1423378 bytes. Are you sure you tested the correct branch?



#6 DrVenkman ONLINE  

DrVenkman

    River Patroller

  • 3,983 posts
  • Back off, man! I'm a scientist.
  • Location:KMBT

Posted Sat May 7, 2016 8:14 PM

 

 

Strange, I just did clean builds of both the master branch and my blind-fixes branch and got a size decrease (of the main binary, not including frameworks and other bundled files) from 1753084 bytes to 1423378 bytes. Are you sure you tested the correct branch?

 

 

I built the branch you linked in your post. But if you don't included frameworks and bundled files  (e.g., don't deploy it) your binary can't be used on any machine other than your own. I build releases to upload back to Github, so I can't count on folks have Qt installed.



#7 blind_io OFFLINE  

blind_io

    Combat Commando

  • Topic Starter
  • 8 posts
  • Location:Stockholm, Sweden

Posted Sun May 8, 2016 4:29 AM

 

 

I built the branch you linked in your post. But if you don't included frameworks and bundled files  (e.g., don't deploy it) your binary can't be used on any machine other than your own. I build releases to upload back to Github, so I can't count on folks have Qt installed.

 

 

The point I was trying to make about the binary size was to show how much unnecessary stuff there was in the UI code. From that point of view, the size of the Qt frameworks and other bundled files are irrelevant. I never claimed that the final app-bundle would be 20% smaller. :)



#8 TheMontezuma OFFLINE  

TheMontezuma

    Dragonstomper

  • 676 posts
  • Location:Hildesheim, D / Kraków, PL

Posted Sun May 8, 2016 5:15 AM

@DrVenkman

I'm pretty sure that you have downloaded the source code from the 'master' branch.

Below is the link to the 'blind-fixes' branch:

https://github.com/b...ree/blind-fixes

And the binary is really smaller.



#9 DrVenkman ONLINE  

DrVenkman

    River Patroller

  • 3,983 posts
  • Back off, man! I'm a scientist.
  • Location:KMBT

Posted Sun May 8, 2016 9:20 AM

@DrVenkman

I'm pretty sure that you have downloaded the source code from the 'master' branch.

Below is the link to the 'blind-fixes' branch:

https://github.com/b...ree/blind-fixes

And the binary is really smaller.

 

 

Just to be sure, I pulled another copy of the blind fixes branch and built the binary again. I get essentially the same size file as I've always gotten before deployment, about 1.9MB (1,911,621 bytes to be precise), as compared to about 2.1MB with the r3 branch. So yeah, couple hundred kilobytes smaller, but then again, when you add the 23+ MB of frameworks and secondary boot folders into the executable bundle, it's not a very big difference. 


Edited by DrVenkman, Sun May 8, 2016 9:29 AM.


#10 Joey Z OFFLINE  

Joey Z

    Dragonstomper

  • 876 posts
  • Location:Hoffman Estates, IL

Posted Thu May 12, 2016 11:47 AM

The only thing different in the main window should be the offset of the widgets in the right column (disks 9 to O). It is a side effect of adding the widgets from code. Since I'm not a Qt professional, I couldn't find a easy way of fixing the alignment. But my intention is to fix them so they align again.

Also I seems to have committed the native placement of the menu bar that have been discussed on github. As a mac user, anything else just feels wrong. I don't really know how that mode affects other systems, and i don't know what is preferred by other users.

 

If you can fix the alignment of the widgets in the main window, then go ahead and do a pull request on github, and I'll have a look.

 

But, please add conditional compilation for the native menu bar, as it has been known to cause issues on platforms which don't support it.



#11 blind_io OFFLINE  

blind_io

    Combat Commando

  • Topic Starter
  • 8 posts
  • Location:Stockholm, Sweden

Posted Fri May 13, 2016 4:12 PM

If you can fix the alignment of the widgets in the main window, then go ahead and do a pull request on github, and I'll have a look.

But, please add conditional compilation for the native menu bar, as it has been known to cause issues on platforms which don't support it.


I'm planning to fix the things I broke in the near future. I found a problem with the boot options dialog that I have already fixed and pushed to my fork. Up next is the "recent files" feature, then it's just the widget alignment to do.

Do you have any more information of what issues the native menu can cause? According to the official docs this setting should only affect OS X and Windows CE builds, other platforms doesn't use this flag.



#12 Joey Z OFFLINE  

Joey Z

    Dragonstomper

  • 876 posts
  • Location:Hoffman Estates, IL

Posted Fri May 13, 2016 5:24 PM

I'm planning to fix the things I broke in the near future. I found a problem with the boot options dialog that I have already fixed and pushed to my fork. Up next is the "recent files" feature, then it's just the widget alignment to do.

Do you have any more information of what issues the native menu can cause? According to the official docs this setting should only affect OS X and Windows CE builds, other platforms doesn't use this flag.

I had it give me no menu bar at all on linux with XFCE. I went and disabled native menu bar support and I then had a menu bar. Clearly, it can affect more than OS X and windows CE, so I'd rather have conditional compilation determine whether it is on or not.



#13 blind_io OFFLINE  

blind_io

    Combat Commando

  • Topic Starter
  • 8 posts
  • Location:Stockholm, Sweden

Posted Tue May 17, 2016 2:47 PM

I had it give me no menu bar at all on linux with XFCE. I went and disabled native menu bar support and I then had a menu bar. Clearly, it can affect more than OS X and windows CE, so I'd rather have conditional compilation determine whether it is on or not.

 

 

Interesting. I'll revert the native menu bar setting before I do the pull request, but since I don't have a solution to the conditional compile we can do what DrVenkman does for his Mac builds and enable it manually before building.

 

My change does however make it more feasible to actually have separate mainwindow.ui files for different platforms. That particular file went from over 8000 lines of xml to under 600. But some sort of conditional at compile time would still be the best solution.



#14 Joey Z OFFLINE  

Joey Z

    Dragonstomper

  • 876 posts
  • Location:Hoffman Estates, IL

Posted Fri Jun 10, 2016 6:47 AM

 

 

Interesting. I'll revert the native menu bar setting before I do the pull request, but since I don't have a solution to the conditional compile we can do what DrVenkman does for his Mac builds and enable it manually before building.

 

My change does however make it more feasible to actually have separate mainwindow.ui files for different platforms. That particular file went from over 8000 lines of xml to under 600. But some sort of conditional at compile time would still be the best solution.

It should be possible to do a simple '#ifdef' block with something like Q_OS_MAC or whatever it is in mainwindow.cpp which has a line that sets the nativemenubar parameter of the menubar to true.



#15 blind_io OFFLINE  

blind_io

    Combat Commando

  • Topic Starter
  • 8 posts
  • Location:Stockholm, Sweden

Posted Sun Mar 19, 2017 10:26 AM

It should be possible to do a simple '#ifdef' block with something like Q_OS_MAC or whatever it is in mainwindow.cpp which has a line that sets the nativemenubar parameter of the menubar to true.

 

 

Hey, I'm back. I tried the #ifdef thing last year, but since the option is saved in the .ui file, it's too late do do anything about it once the code is executing. I'll leave the native option off for now.

 

I've fixed all the other outstanding issues I had after my refactoring, so I'm doing a pull request any day now. :)



#16 Joey Z OFFLINE  

Joey Z

    Dragonstomper

  • 876 posts
  • Location:Hoffman Estates, IL

Posted Tue Apr 11, 2017 2:18 PM

 

 

Hey, I'm back. I tried the #ifdef thing last year, but since the option is saved in the .ui file, it's too late do do anything about it once the code is executing. I'll leave the native option off for now.

 

I've fixed all the other outstanding issues I had after my refactoring, so I'm doing a pull request any day now. :)

No, there's probably a way to do it. All the .ui file stuff is processed into C++ code that sets an option somewhere. You just need to figure out where it sets that option, and what it is, and then figure out where is a safe place to set it during run time in code that isn't produced from the ui file. I'll look into it myself when I have enough time to do so, if this is still an outstanding issue when I get to it.





Reply to this topic



  


0 user(s) are browsing this forum

0 members, 0 guests, 0 anonymous users