| Message ID | E14D0C7B-2C4B-4D29-AC31-A651AB7573DE@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 12/29/2016 11:24 AM, Programmingkid wrote: > Programs running inside of QEMU can sometimes use more CPU time than is really > needed. To solve this problem, we just need to throttle the virtual CPU. This > feature will stop laptops from burning up. > > This patch adds a menu called Speed that has menu items from 10 to 1. They > represent the speed options. 10 is full speed and 1 is slowest. Why not make it a type-in box where the user specifies a percentage, up to 100 (unthrottled)? Is this feature present in the other GUIs, such as the gtk view? I'm reluctant to invent features for the less-tested cocoa interface that are not FIRST present on other interfaces. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> > --- > ui/cocoa.m | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 56 insertions(+) > > +/* Used by the Speed menu items */ > +- (void)adjustSpeed:(id)sender > +{ > + /* 10 has two digits in it - plus the NULL termination at the end */ > + const int max_title_size = 3; > + char menu_item_title[max_title_size]; > + int speed, menu_number, count, item; > + NSMenu *menu; > + NSArray *itemArray; > + > + // uncheck all the menu items in the Speed menu > + menu = [sender menu]; > + if(menu != nil) > + { > + count = [[menu itemArray] count]; TAB damage. > + itemArray = [menu itemArray]; > + for(item = 0; item < count; item++) // unselect each item Space after 'for'. > + [[itemArray objectAtIndex: item] setState: NSOffState]; > + } > + > + // check the menu item > + [sender setState: NSOnState]; > + > + // get the menu number > + snprintf(menu_item_title, max_title_size, "%s", [[sender title] cStringUsingEncoding: NSASCIIStringEncoding]); > + sscanf(menu_item_title, "%d", &menu_number); Eew. That just feels fragile. > + > + /* Calculate the speed */ > + // inverse relationship between menu item number and speed > + speed = -1 * menu_number + 10; > + speed = speed * 10; > + cpu_throttle_set(speed); > + COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), '%'); Again, I think that '100%' makes more intuitive sense than '10' for knowing that my VM is running unthrottled. The amount of math you have to do to transform the menu numbers into the cpu_throttle_set() call is a sign that your mapping is rather complex compared to a more intuitive setup. > +} > + > @end > > > @@ -1316,6 +1353,25 @@ int main (int argc, const char * argv[]) { > [menuItem setSubmenu:menu]; > [[NSApp mainMenu] addItem:menuItem]; > > + // Speed menu > + menu = [[NSMenu alloc] initWithTitle:@"Speed"]; > + menuItem = [[[NSMenuItem alloc] initWithTitle:@"10 (fastest)" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]; > + [menuItem setState: NSOnState]; > + [menu addItem: menuItem]; > + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"9" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; > + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"8" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; And this adds a LOT of menu items, compared to what you could get if a single menu item opens up a dialog box for typing in a number that is validated to be between 1 and 100.
On Dec 29, 2016, at 12:35 PM, Eric Blake wrote: > On 12/29/2016 11:24 AM, Programmingkid wrote: >> Programs running inside of QEMU can sometimes use more CPU time than is really >> needed. To solve this problem, we just need to throttle the virtual CPU. This >> feature will stop laptops from burning up. >> >> This patch adds a menu called Speed that has menu items from 10 to 1. They >> represent the speed options. 10 is full speed and 1 is slowest. > > Why not make it a type-in box where the user specifies a percentage, up > to 100 (unthrottled)? It wouldn't feel right. The user would have to use both the keyboard and the mouse to use that feature. It would be tedious. But then again, I could add a scrollbar to the window, so that is a possibility. > Is this feature present in the other GUIs, such as the gtk view? I'm > reluctant to invent features for the less-tested cocoa interface that > are not FIRST present on other interfaces. I would talk to the maintainer of GTK about adding this feature to it, but getting a hold of him is hard. Maybe my emails aren't reaching him. Or he just doesn't have the time to talk. Would you know of anyone else except Gerd Hoffmann who could help with that? > >> >> Signed-off-by: John Arbuckle <programmingkidx@gmail.com> >> --- >> ui/cocoa.m | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 56 insertions(+) > >> >> +/* Used by the Speed menu items */ >> +- (void)adjustSpeed:(id)sender >> +{ >> + /* 10 has two digits in it - plus the NULL termination at the end */ >> + const int max_title_size = 3; >> + char menu_item_title[max_title_size]; >> + int speed, menu_number, count, item; >> + NSMenu *menu; >> + NSArray *itemArray; >> + >> + // uncheck all the menu items in the Speed menu >> + menu = [sender menu]; >> + if(menu != nil) >> + { >> + count = [[menu itemArray] count]; > > TAB damage. I am surprise the checkpatch program didn't see this. > >> + itemArray = [menu itemArray]; >> + for(item = 0; item < count; item++) // unselect each item > > Space after 'for'. Checkpatch.pl should have seen this. Something must be wrong with it. > >> + [[itemArray objectAtIndex: item] setState: NSOffState]; >> + } >> + >> + // check the menu item >> + [sender setState: NSOnState]; >> + >> + // get the menu number >> + snprintf(menu_item_title, max_title_size, "%s", [[sender title] cStringUsingEncoding: NSASCIIStringEncoding]); >> + sscanf(menu_item_title, "%d", &menu_number); > > Eew. That just feels fragile. I agree. I will use another method that doesn't depend on menu item title. That way, if this program is translated into another language, it will still work. > >> + >> + /* Calculate the speed */ >> + // inverse relationship between menu item number and speed >> + speed = -1 * menu_number + 10; >> + speed = speed * 10; >> + cpu_throttle_set(speed); >> + COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), '%'); > > Again, I think that '100%' makes more intuitive sense than '10' for > knowing that my VM is running unthrottled. The amount of math you have > to do to transform the menu numbers into the cpu_throttle_set() call is > a sign that your mapping is rather complex compared to a more intuitive > setup. If the cocoa.m maintainer preferred I stay with the menu items, would this be more to your liking: Speed ------- 100% 90% 80% 70% 60% 50% 40% 30% 20% 10% 1% > >> +} >> + >> @end >> >> >> @@ -1316,6 +1353,25 @@ int main (int argc, const char * argv[]) { >> [menuItem setSubmenu:menu]; >> [[NSApp mainMenu] addItem:menuItem]; >> >> + // Speed menu >> + menu = [[NSMenu alloc] initWithTitle:@"Speed"]; >> + menuItem = [[[NSMenuItem alloc] initWithTitle:@"10 (fastest)" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]; >> + [menuItem setState: NSOnState]; >> + [menu addItem: menuItem]; >> + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"9" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; >> + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"8" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; > > And this adds a LOT of menu items, compared to what you could get if a > single menu item opens up a dialog box for typing in a number that is > validated to be between 1 and 100. Trying to code a dialog box with an editable text field and a scrollbar in cocoa would take a ton more lines of code. Just look at the code for the make_about_window method in cocoa.m. It only displays a picture and some text and it is around 80 lines long. If someone else comes and says they would prefer a dialog box in place of the menu items, then I will make that patch. Thank you for reviewing my patch.
On 29 December 2016 at 17:24, Programmingkid <programmingkidx@gmail.com> wrote: > Programs running inside of QEMU can sometimes use more CPU time than is really > needed. To solve this problem, we just need to throttle the virtual CPU. This > feature will stop laptops from burning up. > > This patch adds a menu called Speed that has menu items from 10 to 1. They > represent the speed options. 10 is full speed and 1 is slowest. > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com> I don't want to take new features into the Cocoa UI that don't already exist in any of QEMU's other UI frontends, I'm afraid. thanks -- PMM
diff --git a/ui/cocoa.m b/ui/cocoa.m index 26d4a1c..293e910 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -35,6 +35,7 @@ #include "sysemu/blockdev.h" #include "qemu-version.h" #include <Carbon/Carbon.h> +#include "qom/cpu.h" #ifndef MAC_OS_X_VERSION_10_5 #define MAC_OS_X_VERSION_10_5 1050 @@ -828,6 +829,7 @@ QemuCocoaView *cocoaView; - (void)openDocumentation:(NSString *)filename; - (IBAction) do_about_menu_item: (id) sender; - (void)make_about_window; +- (void)adjustSpeed:(id)sender; @end @implementation QemuCocoaAppController @@ -1234,6 +1236,41 @@ QemuCocoaView *cocoaView; [superView addSubview: copyright_label]; } +/* Used by the Speed menu items */ +- (void)adjustSpeed:(id)sender +{ + /* 10 has two digits in it - plus the NULL termination at the end */ + const int max_title_size = 3; + char menu_item_title[max_title_size]; + int speed, menu_number, count, item; + NSMenu *menu; + NSArray *itemArray; + + // uncheck all the menu items in the Speed menu + menu = [sender menu]; + if(menu != nil) + { + count = [[menu itemArray] count]; + itemArray = [menu itemArray]; + for(item = 0; item < count; item++) // unselect each item + [[itemArray objectAtIndex: item] setState: NSOffState]; + } + + // check the menu item + [sender setState: NSOnState]; + + // get the menu number + snprintf(menu_item_title, max_title_size, "%s", [[sender title] cStringUsingEncoding: NSASCIIStringEncoding]); + sscanf(menu_item_title, "%d", &menu_number); + + /* Calculate the speed */ + // inverse relationship between menu item number and speed + speed = -1 * menu_number + 10; + speed = speed * 10; + cpu_throttle_set(speed); + COCOA_DEBUG("cpu throttling at %d%c\n", cpu_throttle_get_percentage(), '%'); +} + @end @@ -1316,6 +1353,25 @@ int main (int argc, const char * argv[]) { [menuItem setSubmenu:menu]; [[NSApp mainMenu] addItem:menuItem]; + // Speed menu + menu = [[NSMenu alloc] initWithTitle:@"Speed"]; + menuItem = [[[NSMenuItem alloc] initWithTitle:@"10 (fastest)" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]; + [menuItem setState: NSOnState]; + [menu addItem: menuItem]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"9" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"8" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"7" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"6" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"5" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"4" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"3" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"2" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"1" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"0 (slowest)" action:@selector(adjustSpeed:) keyEquivalent:@""] autorelease]]; + menuItem = [[[NSMenuItem alloc] initWithTitle:@"Speed" action:nil keyEquivalent:@""] autorelease]; + [menuItem setSubmenu:menu]; + [[NSApp mainMenu] addItem:menuItem]; + // Window menu menu = [[NSMenu alloc] initWithTitle:@"Window"]; [menu addItem: [[[NSMenuItem alloc] initWithTitle:@"Minimize" action:@selector(performMiniaturize:) keyEquivalent:@"m"] autorelease]]; // Miniaturize
Programs running inside of QEMU can sometimes use more CPU time than is really needed. To solve this problem, we just need to throttle the virtual CPU. This feature will stop laptops from burning up. This patch adds a menu called Speed that has menu items from 10 to 1. They represent the speed options. 10 is full speed and 1 is slowest. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- ui/cocoa.m | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)