Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GUI init #55

Merged
merged 21 commits into from
Jul 22, 2021
Merged

GUI init #55

merged 21 commits into from
Jul 22, 2021

Conversation

ScherrerMichael
Copy link
Contributor

@ScherrerMichael ScherrerMichael commented Jul 17, 2021

Please try it out!

To run: do ./perf-rust gui

Please Note:

  • Currently, tests and other programs are not implemented in the GUI. In order for them to work properly, programs will need to provide output in the form of a formatted string. If you would like to see where I plan to implement the perf events, check out the Loaded(state) function and the PerfEvent enums. I feel like once we figure this out, we can make graphics later!
  • Loading and saving are not implemented yet. I figured it is more important to have base functionality first.
  • The style is not final.

I would like to update this PR every time a perf event is added.

@ScherrerMichael ScherrerMichael linked an issue Jul 17, 2021 that may be closed by this pull request
@ScherrerMichael ScherrerMichael added the enhancement New feature or request label Jul 17, 2021
@ScherrerMichael ScherrerMichael changed the title Michael gui GUI init Jul 17, 2021
@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Jul 19, 2021

I pulled down this PR locally and tried building but got this error:

error: failed to run custom build command for `freetype-sys v0.13.1`

The build failed after that, did this happen to you at all?

@Boursler
Copy link
Member

The PR builds fine for me and runs.

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Jul 19, 2021

Ran into two somewhat easy hiccups, and a third which I am currently working on. Some things:

  • cmake must be installed
  • This issue gives a necessary command if the servo-fontconfig-sys crate fails to build.
  • The third error message has to do with cc failing to link. This post on stack overflow describes the exact error message I got. And this github issue also references the same error message.

I'll let you guys know more when I get it working.

@Boursler
Copy link
Member

Thanks. If it requires cmake, that helps me understand part of why it built for me as I have that program installed. Still if that's a dependency we should document it somewhere. Is the linker issue mac specific?

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Jul 19, 2021

Nope! Used my Ubuntu machine.

@ScherrerMichael
Copy link
Contributor Author

ScherrerMichael commented Jul 20, 2021

@Sl1mb0 let me know if you still are getting issues. I did remember getting something very similar to that, with the same error message. I'm going to look through it again and see what I did to fix it.

Thanks for recording that!

Edit: Have you tried installing these packages?

sudo apt install pkg-config libasound2-dev libssl-dev cmake libfreetype6-dev
libexpat1-dev libxcb-composite0-dev

source

@Sl1mb0
Copy link
Contributor

Sl1mb0 commented Jul 20, 2021

I have built it successfully. In addition to the dependencies listed previously there are also these dependencies that I installed:

libgtk-3-dev
libx11-dev

source

Edit:

Not sure how we should address these issues for our our project, I just know that we should include something about these dependencies somewhere. I got this message when running target/debug/perf-rust gui:

INTEL-MESA: warning: Ivy Bridge Vulkan support is incomplete

But other than that it opened up fine. It looks awesome @ScherrerMichael, great job!

Copy link
Contributor

@Sl1mb0 Sl1mb0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing I would suggest is breaking up src/gui/mod.rs into separate, smaller, sub-modules so that navigating the gui module is easier.

Just spitballing, but you could maybe do something like this:

src/gui/state.rs
src/gui/content.rs
...

@ScherrerMichael
Copy link
Contributor Author

Sounds good @Sl1mb0, and @Boursler. I was just thinking that too. I'll push out an update tonight with all of these suggestions in mind.

@kagutaba256
Copy link
Contributor

Cool!! This worked for me, but I'm pretty sure I had everything above already installed on my computer.
I like the layout, and seems to be a good system for showing the output of the program. Nice!

I did notice when resizing the window to be pretty small, the "Launch" button loses its text, but stays near the bottom of the pane and turns comically tiny-- maybe we could find out how to keep the button text when it shrinks.

Other than that, I don't love how everything is in mod.rs... a 500 line file is simply too long. I bet we could break it into a few different files.

This is so awesome! Great work on this and thank you!

@ScherrerMichael
Copy link
Contributor Author

I separated the mod.rs into smaller parts as suggested by @Sl1mb0 & @kagutaba256. In addition I added main gui.rs file to the root directory as it follows current file structure guidelines. Will be working on resizing issues noted by @kagutaba256.

Thanks everyone!

Copy link
Member

@Boursler Boursler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for doing this!

@ScherrerMichael ScherrerMichael merged commit 415b1fb into HOMS-OSS:dev Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Init GUI Support
4 participants